Deallocationg char** inside a struct

Question

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])'

`list->storage[pos] = (char*)malloc(strlen(string) * sizeof(char)); list->storage[pos] = string;` is nonsense. — Antti Haapala -- Слава Україні, May 15 '19 at 19:28
[don't cast malloc](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) — Barmar, May 15 '19 at 19:32
You need to add 1 to `strlen(string)`, to add space for the trailing null byte. — Barmar, May 15 '19 at 19:33
You have to use `strcpy()` to copy the string into the allocated space. — Barmar, May 15 '19 at 19:33
Why are you passing a `List **` to `delete_list` rather than just a `List *`? The extra level of indirection seems pointless. — Tom Karzes, May 15 '19 at 19:37
@AnttiHaapala yes you absolute right, this was standing there first, but with some testing i tried it also only copy it. — Tobias M., May 15 '19 at 20:23
@Barmar thanks for this tip, i have deleted the cast's and also to add +1 to the strlen — Tobias M., May 15 '19 at 20:23

Support Ukraine · Accepted Answer · 2019-05-15T19:44:34.600

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

thanks for your fast and great answer. And also for your usefull tips. But i have a further question: then the storage is to small, i will douple it. For example it has size two and two elements are in it. Then i want to add a third item, it should be reallocated with size 4. I done this with the line: 'list->storage = realloc(list->storage, size * 2);' it seem to work so far, but then i want to free this memory i get an error. Make i were something wrong? — Tobias M., May 15 '19 at 20:16

Deallocationg char** inside a struct

1 Answers1