c++ segmentation fault with vector

[gamefanatic]

Hi everyone, I am just a little stuck on a learning exercise that I almost have figured out.

The code I am working on, and posting below, it has an array of suites and array for each card of a suit (for a normal 52 card playing deck of poker)

then it creates an unshuffled vector of a card class pointer which holds just a suite and card.

then I make an infinite loop that doesnt stop until it created a shuffled version of the unshuffled deck.

and finally it outputs the shuffled deck..

it seems like it is working now, for awhile I was having a problem with only part of the shuffled deck printing out, but now all 52 cards print out. BUT still there is a segmentation fault and the program crashes.

can anyone help me figure out why it is crashing?

#include <iostream>
#include <vector>
#include <cstdlib>
#include <ctime>

using namespace std;

class Card {
    public:
        Card();

        void init(int suite, int card);
        int getSuit() { return suite; }
        int getCard() { return card; }
        void clean() { delete this; }
    private:
        int suite;
        int card;
};
Card::Card() {}
void Card::init(int suite, int card) {
    Card::suite = suite;
    Card::card = card;
}

int main()
{
    int suites[4] = {1,2,3,4};
    int cards[13] = {1,2,3,4,5,6,7,8,9,10,11,12,13};

    vector<Card*> deck;
    vector<Card*> shuffledDeck;

    for(int i = 0; i <= 3; i++) {
        for(int j = 0; j <= 12; j++) {
            Card* newCard;
            newCard = new Card();
            newCard->init(suites[i], cards[j]);
            deck.push_back(newCard);
        }
    }

    cout << " Unshuffled deck: " << endl;

    for(unsigned int i = 0; i < deck.size(); i++) {
        cout << deck[i]->getSuit() << " " << deck[i]->getCard() << endl;
    }

    cout << " Shuffle deck " << endl;

    bool shuffling = true;
    int newCard;
    srand(time(NULL));
    while(shuffling) {
        newCard = rand() % 52;// 0 - 51
        //if(deck[newCard] == NULL) continue;
        bool cardExists = false;
        if(!shuffledDeck.empty()) {
            for(int i = 0; i <= shuffledDeck.size(); i++) {
                if(shuffledDeck[i] == deck[newCard]) {
                    cardExists = true;
                    break;
                }
            }
        }
        if(cardExists) continue;

        //cout << "attempting to push back deck[" << newCard << "] " << deck[newCard] << endl;
        shuffledDeck.push_back(deck[newCard]);

        if(shuffledDeck.size() == deck.size())
        shuffling = false;

    }

    cout << endl << " sizes: " << deck.size() << " " << shuffledDeck.size() << endl;

    cout << endl << "Show Shuffled Deck " << endl;
    for(unsigned int i = 0; i <= shuffledDeck.size(); i++) {
        if(shuffledDeck[i] == NULL) continue;
        cout << shuffledDeck[i]->getSuit() << " " << shuffledDeck[i]->getCard() << endl;
    }


    while(!shuffledDeck.empty()) {
        shuffledDeck.back()->clean();
        shuffledDeck.pop_back();
    }

    while(!deck.empty()) {
        deck.back()->clean();
        deck.pop_back();
    }
    
    return 0;
}

[Moschops]

Time to learn to debug! Lines beginning (gdb) is something I typed. The rest is debugger output. The following took less than sixty seconds...

(gdb) run
Starting program: ./a.exe
[New thread 4568.0x1750]
 Unshuffled deck:
1 1
1 2
1 3
1 4
1 5
1 6
1 7
1 8
1 9
1 10
1 11
1 12
1 13
2 1
2 2
2 3
2 4
2 5
2 6
2 7
2 8
2 9
2 10
2 11
2 12
2 13
3 1
3 2
3 3
3 4
3 5
3 6
3 7
3 8
3 9
3 10
3 11
3 12
3 13
4 1
4 2
4 3
4 4
4 5
4 6
4 7
4 8
4 9
4 10
4 11
4 12
4 13
 Shuffle deck

 sizes: 52 52

Show Shuffled Deck
1 12
1 11
3 7
2 3
2 7
1 9
2 2
3 9
2 10
2 12
4 3
1 6
3 6
1 5
3 8
4 12
4 5
4 8
4 9
4 4
3 4
4 1
3 13
2 6
4 11
2 4
2 9
2 1
1 1
1 4
3 10
3 5
4 2
3 2
3 11
1 3
2 13
3 12
3 1
4 7
4 10
1 13
1 8
4 6
1 10
1 2
2 8
2 11
2 5
1 7
3 3
4 13

Program received signal SIGSEGV, Segmentation fault.
0x0040e34a in Card::getCard (this=0xbaadf00d) at 097.cpp:14
14              int getCard() { return card; }
(gdb) print card
Cannot access memory at address 0xbaadf011
(gdb)

Hmm... looks like there's some kind of problem with that card object. 0xbaadf011... that's suspiciously close to the value 0xbaadf00d, which is used commonly "by Microsoft's LocalAlloc(LMEM_FIXED) to mark uninitialised allocated heap memory" ( http://en.wikipedia.org/wiki/Magic_number_%28programming%29).

Accessing memory that we've never put anything in. Hmm. Let's debug a little further:

(gdb) up
#1  0x004017d6 in main () at 097.cpp:81
81              cout << shuffledDeck[i]->getSuit() << " " << shuffledDeck[i]->ge
tCard() << endl;
(gdb) print i
$1 = 52
(gdb)

So we're tyrying to do something to the object in position 52 in shuffledDeck. That seems suspicious - a deck of cards has 52 cards in it, so in an array, they would occupy positions 0 to 51. Let's double check the size of shuffledDeck:

(gdb) print shuffledDeck.size()
$3 = 52

Well there we have it. shuffledDeck is of size 52, so shuffledDeck[0] to shuffledDeck[51] exists, and shuffledDeck[52] is off the end. That object simply does not exist.

Why are we trying to read number 52?
for(unsigned int i = 0; i <= shuffledDeck.size(); i++) {

[coder777]

The crash happens because you delete Card twice online 86 and 91. shuffledDeck and deck contain pointer to the same thing.

delete this on line 15 is certainly not good either

[gamefanatic]

Thanks for all of this, I have been trying to learn to use the debugger but I don't get it yet so I end up just searching through the code myself.

I tried this, and the program no longer crashes... but it is missing the last card now, only 51 cards print
for(unsigned int i = 0; i <= shuffledDeck.size() - 1; i++)

edit: i have been at this point a couple times in the last few hours

[gamefanatic]

coder777 that makes sense, with them holding a pointer to the same object. But what is wrong with line 15? just the fact that the class is in the main file? i was just doing this as an example

[Moschops]

The more common way to sort that out is this:
for(unsigned int i = 0; i < shuffledDeck.size(); i++)
Don't forget to fix both of those.

Once that's sorted, you've got coder777's find above, in which you delete the same object more than once.

When I run your code, I get all 52 cards out (often accompanied by a crash because I've not done anything with the double delete). Are you sure you don't? Try this line to output the cards and see if i goes from zero to 51 - it does for me.

cout << "i = " << i << shuffledDeck[i]->getSuit() << " " << shuffledDeck[i]->getCard() << endl;

[gamefanatic]

Moschops,

I left the loop like this:
for(unsigned int i = 0; i <= shuffledDeck.size(); i++)

and I tried outputting as you suggested:
cout << " i = " << i << " " << shuffledDeck[i]->getSuit() << " " << shuffledDeck[i]->getCard() << endl;

I am getting 0 - 51 cards with no crash now, I removed the double delete this, that seemed to be the only problem. is there something wrong with my loop? seems like the program works

ahh I see it now, < instead of <= I will give it a try

edit: That works, thanks everyone.

[Moschops]

is there something wrong with my loop?

Which loop? You've got two like that.

A segFault will only be thrown if the OS thinks you're trying to access memory that isn't yours. If you incorrectly read off the end of the array and by chance the OS doesn't object, you've been doubly-unlucky, as the code is bad and you don't know about it. It's worth getting it right, rather than just being lucky. Given that you know you meant to deal with shuffledDeck[0] to shuffledDeck[51], and you also know that
for(unsigned int i = 0; i <= shuffledDeck.size(); i++)
will do something with shuffledDeck[52], not fixing it seems like asking for trouble.

[coder777]

Moschops is right about the second loop

for(unsigned int i = 0; i <= shuffledDeck.size(); i++) is wrong.

You might not get the crash due to line 80.

The reason for not getting 52 cards you find on line 66 and depends on line 55

[gamefanatic]

i saw it right after I asked