C: How to free memory after realloc for string arrays-CodePudding

I'm learning C and want to create a simple function which keeps track of a list of paths.

I start with a default path ["/bin/"] and have a function "change_paths" which updates this list based on some input, so for example ["/bin/"] -> ["/temp/", "/auxillary/"].

However, I encounter problems when I try to free my memory at the end. Specifically, fsanitize tells me that the 6 bytes from my default PATH paths[0] = malloc(6 * sizeof(char)); are not freed properly.

What should I do to correctly free this memory if I use change_paths arbitrarily many times with arbitrarily many inputs (including possibly 0 new PATHs)? Is this structure appropriate for storing data with this mechanism?

Note: I've edited this code for brevity, so even though the line "char** new_paths = {"/temp/", "/auxillary/"};" is not technically correct, please think of it as working. I just want to give an example and I'm not sure how to define a char** manually.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

void change_paths(int *num_paths, char*** paths, int num_new_paths, char** new_paths){
   
    *paths = realloc(*paths, num_new_paths * sizeof(char*));

    for (int i = 0; i < num_new_paths; i  ){
        *paths[i] = malloc((strlen(new_paths[i])   1) * sizeof(char));
        strcpy(*paths[i], new_paths[i]);
        }
}

int main(int argc, char *argv[]){
    // A Default PATH
    char** paths = malloc(1 * sizeof(char*));
    paths[0] = malloc(6 * sizeof(char));                         // <- HOW TO FREE THIS AFTER REALLOC OF paths?
    strcpy(paths[0], "/bin/");
    int num_paths = 1;

    // A new set of PATHs
    int num_new_paths = 2;
    char** new_paths = {"/temp/", "/auxillary/"};
    change_paths(&num_paths, &paths, num_new_paths, new_paths);

    // Free memory
    for(int i = 0; i < num_new_paths; i  ){
        free(paths[i]);
    }
    free(paths);

    return 0;
}

CodePudding user response：

You need to keep track of the number of old path entries, and free each one before you overwrite the reallocated space with new path entries.

void change_paths(int *num_paths, char ***paths, int num_new_paths, char **new_paths)
{
    for (int i = 0; i < *num_paths; i  )
        free((*paths)[i]);

    *paths = realloc(*paths, num_new_paths * sizeof((*paths)[0]));

    for (int i = 0; i < num_new_paths; i  )
    {
        (*paths)[i] = malloc((strlen(new_paths[i])   1));
        strcpy((*paths)[i], new_paths[i]);
    }
    *num_paths = num_new_paths;
}

Consider using strdup() in the second loop:

void change_paths(int *num_paths, char ***paths, int num_new_paths, char **new_paths)
{
    for (int i = 0; i < *num_paths; i  )
        free((*paths)[i]);

    *paths = realloc(*paths, num_new_paths * sizeof((*paths)[0]));

    for (int i = 0; i < num_new_paths; i  )        
        (*paths)[i] = strdup(new_paths[i]));
    *num_paths = num_new_paths;
}

Also, don't forget to error check memory allocations and handle them appropriately. Note that the idiom:

old_space = realloc(old_space, new_size);

leaks memory if the reallocation fails. It is important to use:

void *new_space = realloc(old_space, new_size);
if (new_space == NULL)
    …deal with error…
old_space = new_space;

That leads to this code:

void change_paths(int *num_paths, char ***paths, int num_new_paths, char **new_paths)
{
    for (int i = 0; i < *num_paths; i  )
        free((*paths)[i]);
    *num_paths = 0;  // In case realloc fails

    void *new_space = realloc(*paths, num_new_paths * sizeof((*paths)[0]));
    if (new_space == NULL)
        return;
    *paths = new_space;
    for (int i = 0; i < *num_paths; i  )
        free((*paths)[i]);
    for (int i = 0; i < num_new_paths; i  )        
        (*paths)[i] = strdup(new_paths[i]));
    *num_paths = num_new_paths;
}

Note that if strdup() fails, some of the entries in the array may be null pointers. You could use other error handling for failures in strdup().

^{Warning: no compiler was consulted about the validity of any of this code.}