C (std=c99) pointer to structs memory allocation

Question

So, this is working fine... that means, no compiler errors, it seems that there is no memory leak and it is doing what I wanted it to do. That said should it be working? When I go to books_init I send a local variable to collection, doesn't it mean that when I go back to main it shouldn't be working? (or undefined behavior?). Also, in case you say that I have to malloc it, do I have to free it after? (commented on cleanup)

/* pseudo struct Collection{
    size_t size, capacity;
    Volume *volumes;
} */

void collection_init(Collection *col, size_t capacity){
    col->size = 0;
    col->capacity = capacity;
    col->volumes = malloc(sizeof(Volume) * capacity);
}

void collection_resize(Collection *col, size_t capacity){
    Volume *v = realloc(col->volumes, capacity * sizeof(Volume));
    if(!v) return;
    col->capacity = capacity;
    col->volumes = v;
}

void collection_add(Collection *col, Volume *volume){
    if(col->size >= col->capacity)
        collection_resize(col, col->capacity * 2);
    col->volumes[col->size++] = *volume;
}

void collection_clean(Collection *col){
    //for(vol : col->vol) free(vol);
    //should I free every element or just volumes?
    free(col->volumes);
}

void books_init(Collection *col){
    for(int i = 0; i < 25; ++i){
        Volume v = {.swag = i};
        collection_add(col, &v);
    }
}    



int main(){
    Collection col;
    collection_init(&col, 10); 
    books_init(&col);
    for(int i = 0; i < col.size; ++i){
        printf("\tVol[%d].id = %d\n", i, col.volumes[i].swag);
    }
    collection_clean(&col);
    return 0;
}

Thanks for your time

Answer 1

This line in books_init

Volume v = {.swag = i};

creates a local variable called v with member swag initialized to i. The address of that variable is then passed to collection_add. That is allowed because v is still in scope.

This line in collection_add

col->volumes[col->size++] = *volume;

makes a copy of the contents of the Volume structure, and stores that copy in the memory that was allocated in collection_init.

After collection_add returns, the variable v in books_init goes out of scope, but that's OK because the contents of v were copied and saved in the memory that col->volumes points to.

When the program ends, collection_clean only needs

free(col->volumes);

to remove all of the Volume copies from memory.

The only flaw I see in your program occurs if realloc fails. In that case, you still write to the Volume array. This will cause a buffer overrun and memory corruption. To avoid this, the collection_add function should verify that the collection_resize function succeeded before performing the copy. For example, you could check again that col->capacity > col->size before doing the copy.

TL;DR your code is fine as long as the realloc always succeeds.