9

Okay it probably doesn't. That would be weird. I'm guessing, I'm allocating my memory wrong somehow.

#include <stdio.h>
#include <stdlib.h>

/*I have two Structs. In one there is a pointer member,
that points to the other. Here they are.*/

typedef struct card{
    int value;
} card;

typedef struct player{
    card* hand;
    int value;
} player;

/*To put it on SO, I tried renaming card, player and hand
 to something like foo and bar, but then the error doesn't occur anymore.*/

int main(){

    //Here is what I do:
    player* player = malloc(sizeof(player)*5);
    for(int playerIndex = 0; playerIndex < 5; playerIndex++){
        player[playerIndex].hand = malloc(sizeof(card)*(12));
    }
    //I allocate some memory.

    //And then I try to access it.
    for(int i = 0; i < 12; i++){
        for(int j = 0; j < 5; j++){
            player[j].hand[i].value = 0;
        }
    }

    return 0;
}

I think it's important to note, that removing the second variable from the player struct also fixes the problem.

I have also checked if the pointers returned from malloc() are == NULL. They are not.

1
  • 3
    why you did not did it as sizeof(struct player)? what u suppose sizeof does? Commented 2 days ago

1 Answer 1

18

On this line:

player* player = malloc(sizeof(player)*5);

player refers to two things, a variable and a type. Because you defined a variable with the same name as a type, anytime you use player from that point forward in the current scope it refers to the variable.

This means that sizeof(player) gives you the size of the variable player, which is a pointer and likely either 4 or 8 bytes. Since the type player is as least that big, you're not allocating enough memory for the array you're creating. This means that you end up writing past the end of allocated memory triggering undefined behavior which in your case causes a crash.

As a general rule, don't give variables the same name as a type as that introduces ambiguity and hard to track down bugs like this.

Renaming either the type or the variable so they're not the same will get rid of the problem.

Sign up to request clarification or add additional context in comments.

7 Comments

Image
Thanks. I renamed the player struct to playerStruct.
Image
One of the common patterns is to name a type starting with a capital, and the variable without (i.e. Player for the type and player for the variable).
dedpunkrz, "renamed the player struct to playerStruct" is insufficient. Change code to playerStruct *player = malloc(sizeof player[0] * 5); or the like.
Consider player[playerIndex].hand = malloc(sizeof(card)*(12)); --> player[playerIndex].hand = malloc(sizeof player[playerIndex].hand[0] * 12); too.
Image
If you're going to include struct in the name, simply don't typedef it. This is C not C++, so only struct player is a type name. (Although I'd still recommend making the struct tag different from the variable name to minimize confusion for humans. Capital names for types and lower-case for variables is a good convention like wohstad suggests.)
Image
Wouldn't players be a better name for the variable?
Image
Yes, except using player means that reading player[0] or player[i] aloud / in your head makes it singular. So "player zero" or "player i". Mental shortcut vs. "players index 0", or the discordant "players zero". So I can see the motivation, especially in C where you usually loop manually instead of passing a collection to an algorithm.

Your Answer

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.