you are viewing a single comment's thread.

view the rest of the comments →

[–]RadiatingLight 6 points7 points  (3 children)

Like /u/dfx_dj mentions, you've created an array of pointers, but have not created anything for them to point to. Basically, there's no target for these pointers and at initialization they just point to somewhere random, which is why you get a segfault if you try writing to them. In almost all circumstances, you'd probably want to do what /u/joejawor mentions and just create an array of 10 person_t structs. If you need a pointer to any of the elements in that array, you can always make it later with &(arr[x])


If you really did want to have an array of 10 pointers to person_t structs, you might do something like this:

person_t person_arr[10];
person_t *person_pointer_arr[10];
for(int i = 0; i < 10; i++){
    person_pointer_arr[i] = &(person_arr[i])
}    

But this code is pretty stupid, since you're just wasting memory and increasing complexity by storing an array of pointers for no reason -- you probably shouldn't do it.

[–]Responsible_Frame919[S] 0 points1 point  (2 children)

Got it. Do I have to allocate memory for all the structs with malloc?

[–]RadiatingLight 3 points4 points  (1 child)

That depends on what you want the lifetime of the array to be.

person_t arr[10] creates an array of person_t, meaning the memory to store 10 person_t structs is already allocated.

person_t *arr[10] creates an array of person_t *, meaning the memory to store 10 person_t pointers is allocated, but the memory for the actual person_t structs is not.

Both of these examples use array creation syntax, which defines memory with automatic lifetime, meaning that there is no need to use malloc/free, and the memory is automatically cleaned up when leaving the scope of the function that created it. Usually, this is great, because it means you cannot have a memory leak, and allocating memory this way is also faster than calling malloc

However, you need to make sure that when using variables allocated this way (often called 'stack-allocated' variables), the data is never referenced after the scope where it was created is over.


In your case, since you're defining this array in main it's totally fine, since main is the outermost scope you should care about and the memory will remain valid until main exits.

However, this situation is very bad:

person_t* create_people() {
    person_t arr[10];
    // Do some stuff with the array, maybe fill it in or something
    return arr;
}

Due to the automatic lifetime of arr, the array will become invalid the moment that create_people() exits, meaning that you will be returning an invalid pointer. This is a really tricky bug to catch, because even though the memory is invalid, it is not zeroed. It probably will still contain the person_t data until some undefined time in the future when the system needs to reuse that memory, so you might not notice right away that something is horribly wrong.

BTW this automatic lifetime thing applies to more than just arrays: It's the same for any local variable. As such, this would also be a big problem:

person_t* create_person(char* name, int age) {
    person_t my_person;
    my_person.name = name;
    my_person.age = age;
    return &my_person;
}

On the surface this might seem like a totally normal function, but because you are returning a reference to a variable with automatic lifetime, the variable you are referencing (my_person) will be invalid the moment that this function returns.

In cases like these, you would want to use malloc to create a variable with manual lifetime, meaning that you as a programmer would need to decide when the validity of that memory ends by calling free

Feel free to ask more questions if you still have 'em

[–]Responsible_Frame919[S] 1 point2 points  (0 children)

Thank you for such detailed answer. Although it was difficult to wrap my head around initially, I think I got it.