Ad

Why Can't I Add A Node To Linked List In C?

I have a problem with linked lists in C. I made a function that created a new node of the list with some information (char *description) and added it to its end. The code is following:

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


struct node {
     char *description;
     struct node *next;
};


// The function to create a node and append it to the linked list of nodes

struct node* create_node(struct node *first, char *description) {
     struct node *current = first;

     // Iteration through the list until the end
     while(current != NULL) {
         node++;
         current = current -> next;
     }

     // Now pointer current points at the end of the list, first -> next. How to assign the pointer new to first -> next through current?

     struct node *new = malloc(sizeof(struct container));

     new -> next = NULL;
     new -> description = malloc(sizeof(description));

     memcpy(new -> description, description, sizeof(description));

     current = new;
     return current;
}

int main() {
     // Creating the first node
     struct node *first = create_node(NULL, "First");

     // Creating and appending the second node to the list 
     create_node(first, "Second");


     printf("%d\n", first -> next == NULL); // Prints 1, the newly created node hasn't been appended

     return 0;
}

I searched how to create the list of the kind and saw very similar ways of how to do it. I know that it's something basic and most likely there's a simple solution, but I can't find it. Thanks everyone for respond.

Ad

Answer

For starters the function name create_node is confusing. It is much better to name the function at least like append_node.

The second function parameter should have the qualifier const because the passed string is not changed in the function.

In these statements

 new -> description = malloc(sizeof(description));
 memcpy(new -> description, description, sizeof(description));

you are allocating memory of the size equal either to 8 or to 4 bytes depending on the value of sizeof( char * ) and correspondingly coping this number of bytes.

You have at least to write

 new -> description = malloc( strlen(description));
 memcpy(new -> description, description, strlen(description));

But it would be better if you were copying the whole string.

The function has a bug. It does not append a node to the list because within the function there is changed the local pointer current that is not chained to the list.

Take into account that memory allocation can fail. You should process such a situation.

The function can be safer and simpler if to pass the pointer to the head node by reference.

Here is a demonstrative program.

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

struct node 
{
    char *description;
    struct node *next;
};


// The function to create a node and append it to the linked list of nodes

int append_node( struct node **head, const char *description ) 
{
    struct node *new_node = malloc( sizeof( struct node ) );
    int success = new_node != NULL;

    if ( success )
    {
        new_node->description = malloc( strlen( description ) + 1 );
        success = new_node->description != NULL;

        if ( success )
        {
            strcpy( new_node->description, description );
            new_node->next = NULL;

            while ( *head != NULL )
            {
                head = &( *head )->next;
            }

            *head = new_node;
        }
        else
        {
            free( new_node );
        }
    }

    return success; 
}

int main( void ) 
{
    // Creating the first node
    struct node *head = NULL;

    if ( append_node( &head, "first" ) )
    {
        printf( "%s\n", head->description );
    }       

    return 0;
}

The program output is

first
Ad
source: stackoverflow.com
Ad