C: How to free memory after realloc for string arrays

Question

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;
}

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. You also need to use more parentheses. — Jonathan Leffler
– Jonathan Leffler, Commented Jun 19, 2022 at 23:48
*paths[i] should be (*paths)[i]. Otherwise, (nonexistent) 2nd element of paths will be accessed. — MikeCAT
– MikeCAT, Commented Jun 19, 2022 at 23:50

Jonathan Leffler · Accepted Answer · 2022-06-20 01:27:26Z

2

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.}

edited Jun 20, 2022 at 1:27

answered Jun 19, 2022 at 23:55

Jonathan Leffler

759k145 gold badges961 silver badges1.3k bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

kpz Over a year ago

Thank you! I was able to get it working with the advice you and Mike provided.

outis Over a year ago

Shouldn't the freeing of the old paths happen before the realloc, in case *num_paths > *num_new_paths? It might not cause any bugs in practice, but realloc isn't guaranteed to preserve values if the array is truncated.

Jonathan Leffler Over a year ago

@outis — good point: yes. More particularly, if the array is shrunk, it is no longer legitimate to access the elements after the end of the new size.

Collectives™ on Stack Overflow

C: How to free memory after realloc for string arrays

1 Answer 1

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related