c free memory

Question

Hello,

I have written a problem to practice pointers and allocating memory.

However, I am getting a stack dump when I free the memory. Am I freeing in the correct place.

Is there anything else wrong with my program that could make it unsafe?

many thanks for any advice,

void display_names(char **names_to_display, char **output);

int main(void)
{
    char *names[] = {"Luke", "John", "Peter", 0};
    char **my_names = names;
    char *new_output[1024] = {0};
    size_t i = 0;

    // Print the ordinal names
    while(*my_names)
    {
        printf("Name: %s\n", *my_names++);
    }

    my_names = names; /* Reset */
    display_names(my_names, new_output);

    // Print the updated names
    while(new_output[i])
    {
        printf("Full names: %s\n", new_output[i]);
     i++;
    }

    // Free allocated memory
    free(new_output);

    getchar();

    return 0;
}

void display_names(char **names_to_display, char **output)
{
    while(*names_to_display)
    {   
        *output = (char*) malloc(strlen("FullName: ") + strlen(*names_to_display) + 1);
     if(!*output)
     {
      fprintf(stderr, "Cannot allocate memory");
      exit(1);
     }

        // Copy new output
        sprintf(*output, "FullName: %s", *names_to_display++);
        printf("display_names(): Name: %s\n", *output++);
    }   
}

Answer 1

You didn't allocate the memory for new_output, it was allocated by the compiler. free is for use when you malloc memory at runtime, not for freeing memory allocated by the compiler at compile time.

Your new_output is a local variable and will be 'released' when it goes out of scope, i.e. at the closing brace of the function in which it's declared.

Answer 2

Your problem is that When you say:

free(new_output);

new_output is an array on the stack. It was not allocated with malloc(), so you cannot free it with free(). You need to free the pointers that new_output contains.

Answer 3

the declaration of char* new_display[1024] means that you are declaring an array of 1024 elements, each element is a pointer to char. The array itself is statically allocated here, an array of 1024 element will be reserved on the stack. In your example, you are populating the entries of this array by allocating memory using malloc and setting each element of the array, this is the memory that you should free, and not the statically allocated array itself.

So instead of calling free(new_display), you will need to loop through the array entries and do free(new_display[i]), this way you are freeing only what you have allocated.