linked list implementation gives me "command termination"

Question

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

struct node
{
   int data;
   struct node *link;
};

struct node *add_node(struct node *head, struct node *tail);

int main()
{
    struct node *head = NULL, *tail = NULL;
    add_node(head, tail);

    head->data = 2;


    return 0;
}


struct node *add_node(struct node *head, struct node *tail)
{
       if (head == NULL)
       {
          struct node *new_node = (struct node *) malloc(sizeof(struct node));
          head = new_node;
          return new_node;
       }

}

I am trying to make a function that starts a empty linked list. But when i try to store the value '2' in the data member of the first node it does not work. I have tested this inside the actual function and successfully stored and printed '2', why does this occur outside the function?

What would `printf("head: %p\n", head);` before `head->data = 2;` output? — CristiFati, Aug 03 '21 at 10:42
wow, it gives "(nil)" which means nothing got stored in head — programmerc3981143, Aug 03 '21 at 10:52
i think this may be because the new_node variable got destroyed after the function ended or something — programmerc3981143, Aug 03 '21 at 10:52
No, the memory was not destroyed. It's just *head* was not updated. When passing a (simple) pointer, you can modify its data but not the address itself. To do that you'd need a double pointer (*struct node \*\*ppHead*), but that's an overkill in this case. Simply use function's return value. *tail* is useless (at this point). Note that at the end you should free the memory. Note that there are tons of such examples on the web. — CristiFati, Aug 03 '21 at 10:58
Apart from the issue mentioned above `add_node()` should return something in else case — kuro, Aug 03 '21 at 11:00

score 0 · Answer 1 · edited Aug 03 '21 at 14:37

Your add_node function returns a struct node * but in the main function, there is no variable to receive it. Try this.

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

struct node
{
   int data;
   struct node *link;
};

struct node *add_node(struct node *head, struct node *tail);

int main()
{
    struct node *head = NULL, *tail = NULL;
    head = add_node(head, tail); // notice the change here.
    head->data = 2;
    return 0;
}

struct node *add_node(struct node *head, struct node *tail)
{
    if (head == NULL)
    {
        struct node *new_node = (struct node *) malloc(sizeof(struct node));
        head = new_node;
        return new_node;
    }
}

Or you can change your add_node function to take a reference to a pointer than the pointer itself, like this:

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

struct node
{
   int data;
   struct node *link;
};

struct node *add_node(struct node **head, struct node *tail);

int main()
{
    struct node *head = NULL, *tail = NULL;
    add_node(&head, tail);
    head->data = 2;
    return 0;
}

struct node *add_node(struct node **head, struct node *tail)
{
    if (*head == NULL)
    {
        struct node *new_node = (struct node *) malloc(sizeof(struct node));
        *head = new_node;
        return new_node;
    }
}

Vlad from Moscow · Accepted Answer · 2021-08-03T15:59:23.723

The function add_node deals with copies of the pointers head and tail declared in main because they are passed to the function by value.

So this statement within the function

  head = new_node;

changes the function parameter head but not the original object head declared in main.

Moreover the function does not use its parameter tail.

And the function invokes undefined behavior in case when head is not equal to NULL because in this case it returns nothing.

After this call

add_node(head, tail);

the both pointers, head and tail, are still equal to NULL. And dereferencing a null pointer

head->data = 2;

results in undefined behavior.

So truly speaking the function does not make a sense.

You should define one more structure as for example

struct list
{
    struct node *head;
    struct node *tail;
};

Then in main instead of these declarations

struct node *head = NULL, *tail = NULL;

you need to write for example

struct list lst = { .head = NULL, .tail = NULL };

and the function add_node can be declared and defined the following way

int add_node( struct list *lst, int data )
{
    struct node *new_node = malloc( sizeof( struct node ) );
    int success = new_node != NULL;

    if ( success )
    {
        new_node->data = data;
        new_node->link = NULL;

        if ( lst->tail != NULL )
        {
            lst->tail->link = new_node;
        }
        else
        {
            lst->head = new_node;
        }

        lst->tail = new_node;
    }

    return success;
}

And in main the function can be called for example like

add_node( &lst, 10 );

or

if ( !add_node( &lst, 10 ) )
{
    puts( "Error: not enough memory." );
}

Here is a demonstrative program.

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

struct node
{
   int data;
   struct node *link;
};


struct list
{
    struct node *head;
    struct node *tail;
};


int add_node( struct list *lst, int data )
{
    struct node *new_node = malloc( sizeof( struct node ) );
    int success = new_node != NULL;

    if ( success )
    {
        new_node->data = data;
        new_node->link = NULL;

        if ( lst->tail != NULL )
        {
            lst->tail->link = new_node;
        }
        else
        {
            lst->head = new_node;
        }

        lst->tail = new_node;
    }

    return success;
}

int main(void) 
{
    struct list lst = { .head = NULL, .tail = NULL };
    
    if ( add_node( &lst, 10 ) )
    {
        printf( "The head node contains %d\n", lst.head->data );
        printf( "The tail node contains %d\n", lst.tail->data );
    }
    
    return 0;
}

The program output is

The head node contains 10
The tail node contains 10

space-ninja-x · Answer 3 · 2021-08-04T03:08:45.943

Just like Vlad from Moscow said, you are passing the value of head(which is an address) when you call the function add_node(struct node *head, struct node *tail). So at this line: head = new_node;, you are changing the value(an address) that was received in the parameter and not the contents of the head that's in the main() function.

So what do you do? Well consider the code below:

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


struct node
{
    int data ;
    struct node *nextNode ;         // holds the link to the next node of the linked list
} ;


struct node* createNode(int d)
{
    struct node *new_node = malloc(sizeof(struct node)) ;
    new_node->data = d ;
    new_node->nextNode = NULL ;

    return new_node ;       // returns the address of the newly created node
}


// takes the tail pointer of the linked list and adds a new node to the end of the linked list
struct node* addNode(struct node *tail, int dataOfNewNode)
{
    struct node *new_node = createNode(dataOfNewNode) ;
    
    if(tail != NULL)
    {
        tail->nextNode = new_node ;
    }

    return new_node ;
}


void display(struct node *head)
{
    struct node *temp = head ;

    while(temp != NULL)
    {
        printf("[%d] - ", temp->data) ;
        temp = temp->nextNode ;
    }

    printf("\n") ;
}


int main()
{
    struct node *head, *tail ;

    head = NULL ;
    tail = NULL ;

    head = createNode(2) ;
    tail = head ;           // as there's just one node in the linked list for now

    tail = addNode(tail, 4) ;
    tail = addNode(tail, 6) ;

    display(head) ;

    return 0 ;
}

In the code shown above, I've created a function addNode() which takes the address of the tail of the linked list and adds to it, then it returns the address of the new node. So, the newly added node becomes the new tail of the linked list. Once again, here the value of the tail is being passed when addNode() is called. However if you wish to change the contents of the head pointer in the main() function itself, have a look at the 2nd code posted by @susanth29.

You might find this helpful.

linked list implementation gives me "command termination"

3 Answers3