2

I have a problem when trying to initialize my nested structures.

On the compilation, there is no errors, but on the execution, I'm getting a segfault. In valgrind, I get

> Invalid read of size 8  at 0x402175: new_animal  
  > Address 0x38 is not stack'd, malloc'd or (recently) free'd

Here is the code :

    void new_animal(int i, int j, int species){

   struct animal * a;

    if (array[i][j]==NULL) 
    {
        a = malloc(sizeof(struct animal));
        assert (a);
        a->espece=espece;
        if(array[i][j]->player==NULL)
        {
        a->player->id_fisherman=0;
        strcpy(a->player->Name,"N"); //I want it set to NULL.
        }
    }
   grille[i][j] = a;
  }

And here is the two structures :

struct fisherman {
    int id_fisherman;
    char Name[10];
};

struct animal {
    int species;
    struct fisherman* player;
};

It worked fine until I added the fisherman. I don't know if it is due to the memory allocation, or when I'm initializing.

2
  • 1
    And this has nothing to do with structures being "nested". Commented Jan 17, 2015 at 10:53
  • one of two: or you declare player field as a struct fisherman only (without the *) or you do another malloc() to it, or initialize the pointer to somewhat valid (the address of something). Commented Jan 19, 2015 at 6:11

3 Answers 3

4
a->player = malloc(sizeof(struct fisherman));

You need to allocate memory to struct Fisherman before writing something to it

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

Comments

3

One of the problems is here:

if (array[i][j]==NULL) {
    // ... then ...
    if(array[i][j]->player==NULL)
}

If array[i][j] is NULL, then you can't dereference it. You must first assign it to a valid pointer.

Comments

0

Let's break the explanation into parts, following the code.

When you do a = malloc(sizeof(struct animal));, you have a structure with 0 on the species attribute, and memory garbage on the player attribute, for it's is an uninitialized pointer.

As @Gopi said, you have to initialize this pointer before using it:

a->player = malloc(sizeof(struct fisherman));

Now, the following statment is valid:

a->player->id_fisherman=0;

Before using a pointer, as a->player, is better to assert it. The C compiler is not very reliable on possible run-time error detection. In production code, you can set the compiler to don't assert.

A tip to avoid this type of error is to create an initialization function along with the definition of the structure. You could have written:

struct fisherman {
        int id_fisherman;
        char Name[10];
};

*fisherman malloc_fisherman(){
        return malloc(sizeof(struct fisherman));
}

*fisherman malloc_fisherman(int id){
        struct fisherman *a = malloc(sizeof(struct fisherman));
        a->id_fisherman = id;
        return a;
}

Here, you are creating two functions, one for default creation and other to create a struct with a given argument. The struct animal can rely on those functions to make its own creation functions.

Philip Guo has written a great article on basic C programming good practices. Check it out, specially the section Use assert statements to document and enforce function pre-conditions and other assumptions.

OBS: Isn't the following statement wrong? Isn't the attribute named species and not espece?

a->espece=espece;

1 Comment

You are right, they are called species, and not espece. Forgot to change it when translated.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.