Search code examples
cpointersstructfree

Deallocationg char** inside a struct


This problem is similar to mine.
But deliver no solution for me.

This is only a simplified code for testing and better understanding.
I know that this code doesn't care of problems after the malloc functions.

The code is for saving words in a struct called List, within a char** storage that used as array.

To create the list and add an item works fine.
But the deletion of the list makes problems.

Here is the code:

Declaration of the list:

typedef struct {
    char** storage;
} List;

Main:

int main(){
    int size = 2;
    List* list;
    list = new_list(2);
    add(list, "Hello", 0);
    add(list, "World", 1);
    printf("\nlist->storage[0]: %s", list->storage[0]);
    printf("\nlist->storage[1]: %s",  list->storage[1]);

    delete_list(&list,size);

    return 0;
}

Make a new list:

List* new_list(size) {
    List* listptr = malloc(sizeof(List));
    listptr->storage = (char**)malloc(size * sizeof(char));
    return listptr;
}

Add a string to the list:

void add(List* list, char* string, int pos) {
    list->storage[pos] = (char*)malloc(strlen(string) * sizeof(char));
    list->storage[pos] = string;
}

Delete the list, with all members:

void delete_list(List** list, int size) {
    int a = 0;
    for (a = 0; a < size; a++)
        free((*list)->storage[a]);

    free((*list)->storage);
    free(*list);
}

Here I get an error in the for loop, at the line 'free((*list)->storage[a])'.
The goal is here to delete every allocated string.
If the list has no members, therefore the code run not in the for loop and the 'delte_list' function work well.

So that is my mistake in the line: 'free((*list)->storage[a])'


Solution

  • This allocation is wrong:

    listptr->storage = (char**)malloc(size * sizeof(char));
                                                    ^^^^^
    

    As storage is a char** the sizeof should be sizeof(char*). When you only use sizeof(char) you end up with too little memory and later you write outside the allocated memory.

    Also this line:

    list->storage[pos] = string;
    

    seems wrong.

    Here you probably need a strcpy like:

    strcpy(list->storage[pos], string)
    

    and also add 1 to the malloc for the string termination, i.e.

    malloc((1 + strlen(string)) * sizeof(char));
    

    but notice that sizeof(char) is always 1 so

    malloc(1 + strlen(string));
    

    is fine.

    BTW: A good way of getting your malloc correct is to use "sizeof what_the_variable_points_to". Like:

    char** x = malloc(size * sizeof *x);
                                    ^^
                            Use *x instead of sizeof(char*)
    

    in this way you always get the correct size and avoid bugs due to simple typos.

    As an example from your code:

    List* listptr = malloc(sizeof(List));     // works but
    List* listptr = malloc(sizeof *listptr);  // this is less error prone