0

So im programming a poker game (because im bored) and just setting out the classes and testing it works as i go along and its working perfectly BUT suddenly i add some new code to have an actual deck instead of infinite random cards and i just get this error

*** stack smashing detected ***: <unknown> terminated
Aborted (core dumped)

This is from a g++ compiler on mint 19.3 cinnamon

I looked at other questions that are similar to mine but they seem to be about large amounts of data and i dont really see how thats in my program.

If someone could help out or at least explain the error message that would be great

-thanks

/ my code /

#include <iostream>
#include <stdlib.h>

using namespace std;


class Card{
public:
    static Card* deck;
    static int current;
    char house;
    char value;

    void setTo(Card c){
        house = c.house;
        value = c.value;
    }

    void random(){
        setTo(*(deck + current));
        current++;
    }

    void print(){

        switch (value){
            case 11:
                cout << "jack";
                break;
            case 12:
                cout << "queen";
                break;
            case 13:
                cout << "king";
                break;
            case 14:
                cout << "ace";
                break;
            default:
                cout << (int)value;
                break;
        }

        cout << " of ";
        switch (house){
            case 0:
                cout << "spades";
                break;
            case 1:
                cout << "clubs";
                break;
            case 2:
                cout << "hearts";
                break;
            case 3:
                cout << "diamonds";
                break;
            default:
                cout << "there has been an error, the house is invalid";
                break;
        }
        cout << endl;
    }

    static void CreateDeck(){
        Card cs[52];
        deck = &cs[0];

        int k;
        for(int i = 0;i<4;i++){
            for(int j = 0;j<14;j++){
                k =  (i*13) + j;
                deck[k].house = i;
                deck[k].value = (j+1);
            }
        }
    }

    static void ShuffleDeck()
        int j,k;
        Card t;
        for(int i = 0;i<52;i++){
            j = rand() % 52;
            k = rand() % 52;
            t.setTo(*(deck+j));
            (*(deck+j)).setTo(*(deck+k));
            (*(deck+k)).setTo(t);
        }
    }
};

class Player{
    public:
        int chips;
        Card* hand;
        string pName;

        void initialize(string n){
            chips = 1000;
            pName = n;
            Card cs[2];
            hand = &cs[0];
        }

        void print(){
            cout << "player: " << pName << endl;
            cout << "    ";
            (*hand).print();
            cout << "    ";
            (*(hand +1)).print();
            cout << "    " << chips << " chips" << endl;
            cout << endl;
        }

        void deal(){
            (*hand).random();
            (*(hand+1)).random();
        }

};

class Game{
    public:
        int pot;
        Card* deck;

        void initialize(){
            pot = 0;
            Card c[5];
            deck = &c[0];
        }
};

Card* Card::deck = NULL;
int Card::current = 0;

int main()
{

    srand (time(NULL));
    Card::CreateDeck();
    Card::ShuffleDeck();
    Card b[2];
    b[0].random();
    b[1].random();

    b[0].print();
    b[1].print();
    cout << endl;
    return 0;
}
super
  • 12,335
  • 2
  • 19
  • 29
  • 4
    `deck = &cs[0];` <- `cs` is a local variable and it get destroyed at the end of the function. After that `deck` points at invalid memory. Use a `std::vector` instead of raw pointers. – super Jan 05 '20 at 00:33
  • The error is because you use `deck` even though it's a dangling pointer. That's UB and anything can happen. – super Jan 05 '20 at 00:38
  • 2
    @super, this is not the answer section. – Lightness Races in Orbit Jan 05 '20 at 00:41
  • 1
    Excellent reading: [Can a local variable's memory be accessed outside its scope?](https://stackoverflow.com/questions/6441218/can-a-local-variables-memory-be-accessed-outside-its-scope) – alter_igel Jan 05 '20 at 01:14

1 Answers1

3

Your problem is here in createDeck:

    Card cs[52];
    deck = &cs[0];

You have deck point to a local variable in the function. When the function exits the variable goes out of scope, so attempting to dereference deck invokes undefined behavior.

The simplest fix is to dynamically allocate an array using new:

deck = new Card[52];

And have a cleanup routine to delete [] the memory.

A better way would be to define it as a std::vector:

class Card{
public:
    static std::vector<Card> deck;

...

std::vector<Card> Card::deck(52);

This gives you better control over the memory. You will however need to change any explicit pointer arithmetic and derefernce to array subscript notation (i.e *(deck + x) --> deck[x] since std::vector doesn't support these operators.

Also in createDesk, you're going off the end of the array/vector here:

for(int j = 0;j<14;j++){

You want one less:

for(int j = 0;j<13;j++){
dbush
  • 205,898
  • 23
  • 218
  • 273
  • @LightnessRacesBY-SA3.0 Was working on the "better" solution. Added mention of adding a cleanup routine for the `new` case and gave option to use `std::vector`. – dbush Jan 05 '20 at 00:48
  • Not sure what you're getting at regarding `j` ... that's not used for array indexing? – Lightness Races in Orbit Jan 05 '20 at 00:58
  • @LightnessRacesBY-SA3.0 `*(deck+j)` won't work if `deck` is changed to `std::vector` because it doesn't define `operator+(int)`, at least not on gcc 4.8.5. – dbush Jan 05 '20 at 01:01
  • I'm talking about your complaint regarding the loop counter `j`. It's possible the 13 instead of 14 _was_ a mistake (I haven't tried to decode what that particular magic number actually means), but I can't see how this counts as "going off the end of the array/vector" – Lightness Races in Orbit Jan 05 '20 at 01:04
  • @LightnessRacesBY-SA3.0 Using `14` instead of `13` (i.e. the number of cards in a suit) causes the inner loop to execute 56 times but `desk` only has 52 elements. – dbush Jan 05 '20 at 01:05
  • *gave option to use `std::vector`* – That's not an "option", using a container ist the only sensible thing to do! – J. Doe Jan 05 '20 at 01:07
  • @TedLyngmo https://www.quora.com/What-does-the-only-sensible-thing-mean-in-this-phrase – J. Doe Jan 05 '20 at 01:43
  • @J.Doe Ok, one opinion mandates it - Just to clarify, I agree, using a container **is** the sensible thing to do. – Ted Lyngmo Jan 05 '20 at 01:46
  • If the size is known at compile time, and the logic doesn't expect for size to, ever, change, `std::array` is better option, than `std::vector` (IMO). – Algirdas Preidžius Jan 05 '20 at 02:47
  • @J.Doe Using a vector for this isn't all that sensible – Lightness Races in Orbit Jan 05 '20 at 03:19
  • @LightnessRacesBY-SA3.0 I didn't say "vector", I said "container". – J. Doe Jan 05 '20 at 13:05
  • @J.Doe You were directly responding to a quote about vector, and said that it should be "the only sensible thing to do" rather than "an option", whereas in fact the opposite is true. Shrug. – Lightness Races in Orbit Jan 05 '20 at 15:21