How do I free a returned string value that is dynamically allocated in C?

Question

This is my first post on StackOverFlow, I was writing the code for a linked list in C, when suddently stumble upon a situation i have no solution for (located in dupstring function). This is my corrent code:

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

typedef struct node
{
    char * word;
    struct node * next;
}node; 

void add_node(node ** list, const char * word);
void print_linked_list(node * list);
char * dupstring(const char *s);


// MAIN
//___________________________________________

int main(void)
    
{
    srand(time(0));
    char buffer
    node * linked_list;
    add_node(&linked_list,"ola");
    //add_node(&linked_list,"adeus");
    return 0;
}

//____________________________________________

void add_node(node ** list, const char * word)
{
    node * new_node = malloc(sizeof(node));
    if(new_node == NULL)
    {
        printf("Error creating node");
        free(new_node);
        return;
    }
    char * temp_string = dupstring(word);

    printf("%p", &temp_string);
    printf("%s", temp_string);
    new_node->word = temp_string;
    free(temp_string);
    new_node->next = *list;
    *list = new_node;

}

void print_linked_list(node * list)
{

}

char * dupstring(const char * word)
{
    uint32_t length = strlen(word);
    char * copy = malloc(length + 1);
    for (uint32_t i = 0; i < length + 1; ++i)
    {
        copy[i] = word[i];
    }

    return copy;
}

My question is, every memory that's allocated should be freed at some point, I allocated memory for copy (dynamic string), but i cant free copy before I return it. What would be a good approach to solve this problem

I can't figure a way,that allows me to keep the parameter word of dupstring a const variable.

Hope anyone can help, I'm open to any type of criticism. Cheers to everyone.

Answer 1

Where there is an alloc, there must be a free.
Where there is an add, there should be a remove.
Where the add routine does the alloc, the remove operation does the free.

---

To avoid another function introduction and another call to free, one could do:

typedef struct node {
    struct node * next;
    char word[]; //array of unknown size
} node;

then in add_node

//allocate a block of memory of size:
// sizeof(node)
// + size of word
// + 1 byte for '\0'
node* new_node = malloc(sizeof(node) + strlen(word) + 1);
strcpy(node->word, word); //copy string incl. '\0'
//...

As you can see, there is no longer a need for a second allocation. Calling free(some_node_ptr) will not only free the node object but the whole block which also contains the string (word).

See also: https://en.cppreference.com/w/c/language/array#Arrays_of_unknown_size

Answer 2

For starters you did not initialize the pointer linked_list

node * linked_list;

You need to write

node * linked_list = NULL;

Pay attention to that there is a typo in this line

char buffer

there is missed a semicolon

char buffer;

The function add_node should not output any message. It should report to the caller whether it was successful or not. It is the caller of the function that will decide whether to output a message.

So it is better to declare the function like

int add_node( node **list, const char *word );

In this if statement

if(new_node == NULL)
{
    printf("Error creating node");
    free(new_node);
    return;
}

the call of free is redundant because the pointer new_node is already equal to NULL. No memory was allocated.

This call of printf should be written like

printf("%p\n", ( void * )temp_string);

And you need to check whether memory was allocated for the string. That is within the function dupstring you need to check the result of calling malloc. The function can be written like

char * dupstring( const char *word )
{
    size_t length = strlen( word );
    char *copy = malloc( length + 1 );

    if ( copy != NULL ) strcpy( copy, word );

    return copy;
}

You assigned the pointer new_node->word with the address of the allocated string and at once you deleted the string.

new_node->word = temp_string;
free(temp_string);

So the pointer has an invalid value. Dereferencing it results in undefined behavior.

The function add_node can look the following way

int add_node( node **list, const char *word )
{
    char *temp_string = dupstring( word );
    int success = temp_string != NULL;

    if ( success )
    {
        node *new_node = malloc( sizeof( node ) );
        success = new_node != NULL;
    
        if ( success )
        {
            new_node->word = temp_string;
            new_node->next = *list;
            *list = new_node;
        }
        else
        {
            free( temp_string );
        }
    }

    return success;
}

The function print_linked_list should be declared like

void print_linked_list( const node *list );

If you need a function that frees all the allocated memory then it can look the following way

void clear_linked_list( node **list )
{
    while ( *list )
    {
        node *current = *list;
        *list = ( *list )->next;
        free( current->word );
        free( current );
    }
}    

and be called like

clear_linked_list( &linked_list );