1

I'm new to C++. I do tried to find an answer on the books that I got and search on google but can not find a clue to the root of the problem.

It may be something really stupid. Hope someone could shed some light here I'm copying everything here below:

Hand::ShowHand2 is only working with "myHand.Add(pCard2)" somehow when I use myHand.Add(pCard1) I got the following error:

*** glibc detected *** /home/remy/workspace-C/myPoker/Debug/myPoker: double free or corruption (out): 0x00007fff7723d2d0 ***
======= Backtrace: =========
/lib/x86_64-linux-gnu/libc.so.6(+0x7eb96)[0x7fef4fd1bb96]
/home/remy/workspace-C/myPoker/Debug/myPoker[0x401869]
/home/remy/workspace-C/myPoker/Debug/myPoker[0x40172c]
/home/remy/workspace-C/myPoker/Debug/myPoker[0x401b41]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xed)[0x7fef4fcbe76d]
/home/remy/workspace-C/myPoker/Debug/myPoker[0x400e19]
======= Memory map: ========
00400000-00404000 r-xp 00000000 07:00 875533                             /home/remy/workspace-C/myPoker/Debug/myPoker
00603000-00604000 r--p 00003000 07:00 875533                             /home/remy/workspace-C/myPoker/Debug/myPoker
00604000-00605000 rw-p 00004000 07:00 875533                             /home/remy/workspace-C/myPoker/Debug/myPoker
02534000-02555000 rw-p 00000000 00:00 0                                  [heap]

Here is the full code:

#include <iostream>
#include <string>
#include <vector>
using namespace std;

class Card
{
public:
    enum rank {ace = 1, two, three, four, five, six , seven, eight, nine, ten, jack, queen, king};
    enum suit {club =1 , diamonds, hearts, spades};
    friend ostream& operator    <<  (ostream& os,  Card& aCard);

    Card(rank r = ace, suit s = spades);
    rank m_rank;
    suit m_suit;

};

Card::Card(rank r, suit s){
    m_rank =    r;
    m_suit  =   s;
}

ostream& operator<<(ostream& os,  Card& aCard)
{
    const string RANKS[] = {"0", "A", "2", "3", "4", "5", "6", "7", "8", "9",
                        "10", "J", "Q", "K"};
    const string SUITS[] = {"c", "d", "h", "s"};
    os << RANKS[aCard.m_rank] << SUITS[aCard.m_suit];

    return os;
}

class Hand
{
    public:
    Hand();
    virtual ~Hand();

void Add(Card* pCard);
void Clear();
void ShowHand();
void ShowHand2();

    protected:
    vector<Card*> m_Cards;

};

Hand::Hand(){
    cout    << "hand is created "   << endl;
    m_Cards.reserve(7);
}
Hand::~Hand()
{
    Clear();
}

void Hand::Add(Card* pCard)
{
    cout    <<  "*pCard: "  <<  *pCard  << " is @: "    <<  pCard   <<  endl;
    m_Cards.push_back(pCard);
}

void Hand::Clear(){
    vector<Card*>::iterator iter = m_Cards.begin();
    for (iter = m_Cards.begin(); iter != m_Cards.end(); ++iter)
    {
    delete *iter;
    *iter   =   0;
    }
    m_Cards.clear();
}


void Hand::ShowHand(){

        int k = 1;
        vector<Card*>::iterator iter = m_Cards.begin();
    for (iter = m_Cards.begin() ; iter != m_Cards.end(); ++iter, ++k)
    {
        cout    << "card no "   <<  k   << " is: ";
        cout    <<  **iter  << endl ;
    }

}


void Hand::ShowHand2(){

        vector<Card*>::iterator iter = m_Cards.begin();
        cout << "this hand has "    <<  m_Cards.size()  << " card(s)."<< endl;

        for (iter = m_Cards.begin(); iter != m_Cards.end(); ++iter)
{
    cout    << **iter   <<  endl;
}
}



int main(){

Card c1(static_cast<Card::rank>(11), static_cast<Card::suit>(0));

Card*   pCard1  =   &c1;
Card*   pCard2;
pCard2  =   new Card(static_cast<Card::rank>(12), static_cast<Card::suit>(0));

Hand myHand;

myHand.Add(pCard1);
//  myHand.Add(pCard2);

//  myHand.ShowHand();
myHand.ShowHand2();

cout    <<  "End of Program"    <<  endl;


return 0;
}

Big thanks in advance !

20
  • 3
    @Griwes: People learning a new language should learn about it from the ground up. Telling a C++ beginner to "stop using raw pointers" (in a snarky way I may add) is obtuse and counterproductive. Commented Jun 22, 2013 at 18:40
  • 7
    @EdS., learning smart pointers is far closer to the ground than learning raw pointers and manual memory management.
    – Griwes
    Commented Jun 22, 2013 at 18:42
  • 6
    @EdS. what. Why? Came you come up with one good reason why a beginner should start out learning to use language constructs that experienced programmers in that language try to avoid as much as possible? The only reasons to point beginners towards pointers is that (1) you hate beginners and want them to suffer, or (2) you learned C++ wrong, and think others should too Commented Jun 22, 2013 at 18:42
  • 4
    @EdS. when people learn to drive, do you start out by pushing the car over a cliff so that they'll appreciate the airbag? Do you start out ramming them with another car in the first intersection you can find, just so they'll appreciate traffic lights? Commented Jun 22, 2013 at 18:45
  • 4
    @EdS. I suggest you continue this discussion in the Lounge. Commented Jun 22, 2013 at 18:46

2 Answers 2

3

You add pCard1 to your hand, which is a pointer to a local variable that will be destroyed automatically. However, your ~Hand() will call delete on all cards it has assigned. There you have your double free.

If you use pCard2 you allocated the Card instance via new and are therefore responsible for calling delete on it.

As you can see in the comments you should think about using smart pointers to avoid such problems.

Edit To explain your main problem in more detail: You use a pointer to a local variable which is automatically managed by the compiler. However you then add() it to the hand which takes ownership of the pointed object in that it will delete it, when the hand itself is destructed. This basically leads to having two owners for the same object.

By generating an instance via new there is no owner who takes care of the destruction. However, when you add it to the hand then the hand owns it and will delete it.

3
  • thanks lot @Nobody. actually my 400 pages beginner book did not even mention "smart pointer". I'm googling now...hope c++ is not too smart to slow ppl like me :(
    – Remyx
    Commented Jun 23, 2013 at 10:56
  • forgive me to not entirely understand your answer. when exactly ~Hand() is called ? it's after myHand.Add(pCard1); or myHand.showHand2(); ? or after return 0;
    – Remyx
    Commented Jun 23, 2013 at 11:31
  • @Remyx: No! pCard1 is a pointer to c1, which in turn is automatically deleted, when it goes out of scope, i.e., when the return 0; is reached. The same goes for ~Hand(). It also will be automatically deleted at return 0; and this is where you get your double free. First ~Hand() is called which deletes its pointer that points to c1 and then c1 is deleted again, which triggers the error. Commented Jun 23, 2013 at 13:40
1

Lets take this:

enum suit {club =1 , diamonds, hearts, spades};

Here you declare club to be 1, diamonds to be 2,`hearts to be 3 and spades to be 4.

Then you have this array

const string SUITS[] = {"c", "d", "h", "s"};

that you index with the suit.

However, arrays start their index with zero, which means that for a spade you index out of bounds of the array, and enter the territory of undefined behavior when you use the "string" at SUITS[4].

1
  • thanks @Joachim Pileborg. I now changed the code to : const string SUITS[] = {"0","c", "d", "h", "s"};
    – Remyx
    Commented Jun 23, 2013 at 11:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.