Random value in struct field

Question

I'm trying to learn some C and I've started with a simple deck of cards program.

The card is struct containing two ints for face and suit. And I'm building a deck by allocating 52 cards, and going thorough a loop.

card *Deck = (card*)malloc(sizeof(card) * 52);
for(int i = 0; i < 4; i++){
    for(int j = 1; j < 14; j++){
        Deck->face = j;
        Deck->suit = i;
        Deck += sizeof(card);
    }
}
Deck -= sizeof(card) * 52;
printDeck(Deck);

free(Deck);

I have a printCard function that takes a single pointer and prints the values, and a print deck function that calls printCard 52 times. If i print the cards as they are being created, everything goes well, but if i print the cards after all have been created for some reason I get a segfault at the 6th card. I fired up gdb and noticed this output:

Seven of Diamonds<br>
(null) of Diamonds<br>
Nine of Diamonds<br>

The rest of the output is normal. When checking for the value of the 6th card i got:

(gdb) p c->face
$10 = 2675

If i am skipping the 6th card the output is fine and everything works the way it should. I'm new to C but i can't really see how that value got put in that field in my code.

struct card{
    int face;
    int suit;
};

void printCard(card* c){
    printf("%s of %s\n", face_str[c->face], suit_str[c->suit]);
}

Edit:

The printDeck code:

Before:

void printDeck(card* c){
    for(int i = 0; i < 52; i++){
        printCard(c);
        c += sizeof(card);
    }
}

After:

void printDeck(card* c){
    for(int i = 0; i < 52; i++){
        printCard(c);
        c++;
    }
}

printCard didn't changed:

void printCard(card* c){
    printf("%s of %s\n", face_str[c->face], suit_str[c->suit]);
}

[Do not cast the return value of `malloc()`](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc). — Marco Bonelli, Jan 16 '20 at 13:40

Marco Bonelli · Accepted Answer · 2020-01-16T13:57:57.313

The problem in your code is this line:

Deck += sizeof(card);

Here, Deck is a card * (pointer to card). If you want to advance of one position, pointer arithmetic does the job for you, and doing Deck + 1 already advances sizeof(card) ahead. Adding or subtracting from a pointer p always advances sizeof(*p).

The correct code would be:

Deck += 1;
// or 
Deck++;

The same mistake is done after the loop, here:

Deck -= sizeof(card) * 52;

Which should be:

Deck -= 52;

I'm new to C but i can't really see how that value got put in that field in my code.

When you wrongly modify the variable Deck, you end up accessing memory that is beyond the size of the allocated chunk (which is sizeof(card) * 52). This is undefined behavior in C, and therefore trying to print such a value could result in random values being printed (if not worse).

I would suggest you to completely avoid modifying the value of Deck, and instead index it like an array:

card *Deck = malloc(sizeof(card) * 52);

for(int i = 0; i < 4; i++){
    for(int j = 1; j < 14; j++){
        Deck[i*13 + j]->face = j;
        Deck[i*13 + j]->suit = i;
    }
}

printDeck(Deck);
free(Deck);

It could as well be `for (int i = 0; i < 52; i++) { Deck[i]->suit = i%4; Deck[i]->face = i/4+1; }` — chmike, Jan 16 '20 at 13:55

chmike · Answer 2 · 2020-01-16T14:49:19.620

5

It would be interesting to show us the code of the printDeck function.

The problem you have in the given code is Deck += sizeof(card). You assumed that this would move Deck sizeof(card) bytes further. But that is not correct. When Deck is a pointer on card, incrementing Deck by one will increment its address by sizeof(card) bytes.

So your code should be

card *Deck = malloc(sizeof(card) * 52);
for(int i = 0; i < 4; i++){
    for(int j = 1; j < 14; j++){
        Deck->face = j;
        Deck->suit = i;
        Deck++;
    }
}
Deck -= 52;
printDeck(Deck);

free(Deck);

You might have made the same error in printDeck.

edited Jan 16 '20 at 14:49

answered Jan 16 '20 at 13:46

chmike

20,922
21
83
106

"_incrementing Deck by one will increment its address by sizeof(char) bytes._" => incrementing Deck by one will increment its address by sizeof(**card**) bytes. This isn't helped by the very topsy-turvy use of Capitals for instances and small letters for types... :S – underscore_d Jan 16 '20 at 13:49
@underscore_d oops, sorry. You are right. I’ll fix that. – chmike Jan 16 '20 at 13:50
That fixed it! Changing from Deck += sizeof(card) to Deck++ resolved the problem. But i still don't understand why i was getting the random value in the 6th card. It was always that card the rest of the cards were printed properly – vlad27 Jan 16 '20 at 13:54
@vlad27 Undefined behaviour is undefined. Accessing memory out of bounds invokes undefined behaviour. It's not interesting or useful to worry about why some particular manifestation of UB occurs. – underscore_d Jan 16 '20 at 13:55
@vlad27 there might be a problem in the code of `printDeck` too. Until we can see its code, we can’t provide an explanation of what you saw. – chmike Jan 16 '20 at 13:58
@KonradRudolph you are right. I simply copy pasted the code and fixed the bug. I'll fix that mistake. – chmike Jan 16 '20 at 14:48

Random value in struct field

2 Answers2