Reputation: 1
I have managed to solve the problem, using this guide: https://web.stanford.edu/class/cs107/guide_valgrind.html
this is the first project I'm asked to use valgrind and cpp. I have some errors in valgrind that I can't solve. so the first is this:
==2788== Block was alloc'd at
==2788== at 0x4C2E0EF: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==2788== by 0x408F33: __gnu_cxx::new_allocator<std::_Rb_tree_node<std::pair<int const, std::vector<Card*, std::allocator<Card*> > > > >::allocate(unsigned long, void const*) (new_allocator.h:104)
==2788== by 0x408ADB: std::allocator_traits<std::allocator<std::_Rb_tree_node<std::pair<int const, std::vector<Card*, std::allocator<Card*> > > > > >::allocate(std::allocator<std::_Rb_tree_node<std::pair<int const, std::vector<Card*, std::allocator<Card*> > > > >&, unsigned long) (alloc_traits.h:491)
==2788== by 0x40829C: std::_Rb_tree<int, std::pair<int const, std::vector<Card*, std::allocator<Card*> > >, std::_Select1st<std::pair<int const, std::vector<Card*, std::allocator<Card*> > > >, std::less<int>, std::allocator<std::pair<int const, std::vector<Card*, std::allocator<Card*> > > > >::_M_get_node() (stl_tree.h:491)
==2788== by 0x407360: std::_Rb_tree_node<std::pair<int const, std::vector<Card*, std::allocator<Card*> > > >* std::_Rb_tree<int, std::pair<int const, std::vector<Card*, std::allocator<Card*> > >, std::_Select1st<std::pair<int const, std::vector<Card*, std::allocator<Card*> > > >, std::less<int>, std::allocator<std::pair<int const, std::vector<Card*, std::allocator<Card*> > > > >::_M_create_node<std::piecewise_construct_t const&, std::tuple<int const&>, std::tuple<> >(std::piecewise_construct_t const&, std::tuple<int const&>&&, std::tuple<>&&) (stl_tree.h:545)
==2788== by 0x4068E8: std::_Rb_tree_iterator<std::pair<int const, std::vector<Card*, std::allocator<Card*> > > > std::_Rb_tree<int, std::pair<int const, std::vector<Card*, std::allocator<Card*> > >, std::_Select1st<std::pair<int const, std::vector<Card*, std::allocator<Card*> > > >, std::less<int>, std::allocator<std::pair<int const, std::vector<Card*, std::allocator<Card*> > > > >::_M_emplace_hint_unique<std::piecewise_construct_t const&, std::tuple<int const&>, std::tuple<> >(std::_Rb_tree_const_iterator<std::pair<int const, std::vector<Card*, std::allocator<Card*> > > >, std::piecewise_construct_t const&, std::tuple<int const&>&&, std::tuple<>&&) (stl_tree.h:2170)
==2788== by 0x406052: std::map<int, std::vector<Card*, std::allocator<Card*> >, std::less<int>, std::allocator<std::pair<int const, std::vector<Card*, std::allocator<Card*> > > > >::operator[](int const&) (stl_map.h:483)
==2788== by 0x4054A5: Hand::addCard(Card&) (Hand.cpp:186)
==2788== by 0x409760: Player::addCard(Card&) (Player.cpp:17)
==2788== by 0x40B9F5: Game::init() (Game.cpp:175)
==2788== by 0x40ECDB: main (reviiyot.cpp:17)
now the relevant code here is the one below. this function is supposed to add a card to a hand which is a map with a key and a vector of pointers to Card object.
bool Hand::addCard(Card &card){
int key=getKey(card);
vector<Card*>::iterator it_vec;
map <int,vector<Card*>>::iterator it=hand.find(key);
if(it!=hand.end()){ // found in hand
for (it_vec=it->second.begin();it_vec!=it->second.end();it_vec++){
if (card.getShape()<(*it_vec)->getShape()){
hand[key].insert(it_vec,&card);
break;
}
}
if(it_vec==it->second.end()){
hand[key].push_back(&card);
}
}
else {
hand[key]={};
hand[key].push_back(&card);// erorr line
}
return true;
}
now the project is working and there are no leaks, yet valgrind does not seem to like this insertion of a card that is not already in the hand. i assumed it is syntax thing, but I have tried different ways to do that, and it didn't work. i tried to use insert with pair, and also swap with a vector . non seem to work.
here is another error:
==2788== 214 errors in context 5 of 5:
==2788== Invalid read of size 8
==2788== at 0x405EAC: std::vector<Card*, std::allocator<Card*> >::clear() (stl_vector.h:1212)
==2788== by 0x40C02D: Game::play() (Game.cpp:271)
==2788== by 0x40ED00: main (reviiyot.cpp:20)
and here is the code in game (i have marked the line):
void Game:: play(){
Card* curr_card;
int curr_tact;
unsigned int i=0;
string card_first="";
vector<Card*>::iterator iter;
int curr_que;
int num_from_type;
bool que;
int win_que=-1;
int win_asker=-1;
vector<Card*>* card2pass;
for( i=0; i<players.size(); i++){
curr_card=players.at(i)->strategy();
curr_tact=players.at(i)->getTactic();
switch (curr_tact){
case (1):{
curr_que=most_cards(i);
}break;
case (2):{
curr_que=most_cards(i);
}break;
case (3):{
curr_que=players.at(i)->get_curr_que();
}break;
case (4):{
curr_que=players.at(i)->get_curr_que();
}break;
}
card_first=curr_card->toString().at(0);
if (verbal_on==1){
cout <<"Turn " << turn <<endl;
printState();
cout<<players.at(i)->getName()+" asked "+players.at(curr_que)->getName()+" for the value "+card_first << endl <<endl;
}
//here comes the play:
que=players.at(curr_que)->search(*curr_card);
// complete else of this if
if(que==true){
//instead this vec take from players hand
card2pass=players.at(curr_que)->get_cards(*curr_card);
num_from_type=card2pass->size();
players.at(i)->recive(*card2pass);
players.at(curr_que)->removeCard(*curr_card);
// has asker completed a set
completed_set(*curr_card,i);
card2pass->clear(); // error line
// the code continues here..
I used the clear() func for card2pass so I could use it in the next iteration. but for some reason valgrind does not like it.obviously I don't want to delete the cards because I want the other player to get the card so they can point at it. the last one is this one:
**
==2788== 133 errors in context 2 of 5:
==2788== Invalid read of size 8
==2788== at 0x4EDEA60: std::_Rb_tree_increment(std::_Rb_tree_node_base const*) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21)
==2788== by 0x405DE7: std::_Rb_tree_iterator<std::pair<int const, std::vector<Card*, std::allocator<Card*> > > >::operator++(int) (stl_tree.h:213)
==2788== by 0x404D8D: Hand::~Hand() (Hand.cpp:82)
==2788== by 0x4096FF: Player::~Player() (Player.cpp:14)
==2788== by 0x409AED: PlayerType1::~PlayerType1() (Player.cpp:61)
==2788== by 0x409B1D: PlayerType1::~PlayerType1() (Player.cpp:62)
==2788== by 0x40A8EB: Game::~Game() (Game.cpp:31)
==2788== by 0x40EDC9: main (reviiyot.cpp:19)
so Game has these parameters:
class Game {
private:
vector<Player*> players; //The list of the players
Deck deck; //The deck of the game
string winners; //String representing the winners
int verbal_on; // printing command
int turn; // counts the number of turns
the destructor of game:
Game:: ~Game(){
unsigned int i;
if(players.size()!=0){
for(i=0;i<players.size();i++){
delete players[i];
}
}
}
the destructor of the players are empty, (playerType1 extends Player extands the abstract class Hand), Hands destructor is:
Hand:: ~Hand(){
map <int,vector<Card*>>::iterator it_map;
unsigned int it_vec;
for (it_map = hand.begin(); it_map != hand.end(); it_map++){//error line
for (it_vec = 0; it_vec < (it_map->second).size(); it_vec++){
delete (it_map->second).at(it_vec);
}hand.erase(it_map);
}
}
I guess there is some problem with the iterator, but I don't understand what is it.
Upvotes: 0
Views: 319
Reputation: 7198
map <int,vector<Card*>>
bool Hand::addCard(Card &card)
hand[key].push_back(&card)
You use a map with 'pointers to Card'. But pass a reference (Card&) and push_back the address of that reference.
Try
map <int,vector<Card*>>
bool Hand::addCard(Card* card)
hand[key].push_back(card)
or
map <int,vector<Card>>
bool Hand::addCard(Card &card)
hand[key].push_back(card)
Upvotes: 1