unable to understand Null pointer in c

Question

I am working on singly linked list and unable to solve problem (I think the problem is in add function something with NULL pointer), the problem is it only adding first number to list and skipping rest of call to add function.

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

struct node
{
  int i;
  struct node* next;
};

struct node* head = NULL;

int add(int val)
{
  if(head == NULL)
  {
    head = (struct node*)malloc(sizeof(struct node));
    head->i = val;
  }
  else
  {
    struct node* current = head;
    while(current != NULL)
    {
      current = current->next;
    }
    current = (struct node*)malloc(sizeof(struct node));
    current->i = val;
  }
}

int print(struct node* first)
{
  while(first != NULL)
  {
    printf("%d\n",first->i);
    first = first->next;
  }
}

int main()
{
  add(36);
  add(46);
  add(97);
  print(head);
  return 0;
}

@underscore_d That's called C++, and it's irrelevant here, if you want to try to stop people asking about linked lists take it up on meta. — user253751, Aug 01 '16 at 23:47

score 2 · Answer 1 · edited May 23 '17 at 12:00

You are never setting node->next to NULL (for both head->next and current->next paths). Allocating using malloc does not clear the allocated memory, so you need to do this yourself.

Also, when you are adding the second element, you are iterating until current reaches NULL, but you never set previous_node->next to point to new element, so you never actually "link" anything in your linked list.

Also, you shouldn't cast the result of malloc in C.

score 2 · Accepted Answer · answered Aug 01 '16 at 21:05

Two issues here. First, when creating a new node you don't set next to NULL. So when you iterate through the list, you end up reading garbage data, invoking undefined behavior.

Then next problem is that when you iterate through a non-empty list, you "fall off" the end of the list, so current is NULL at the end of the loop and there's no link to the end of the list. You need to stop when current->next is null, then create the new node there.

void add(int val)     // change return type to void since nothing is being returned
{
  if(head == NULL)
  {
    head = malloc(sizeof(struct node));  // don't cast return value of malloc
    head->i = val;
    head->next = NULL;                   // terminate list
  }
  else
  {
    struct node* current = head;
    while(current->next != NULL)         // loop until you're on the last node
    {
      current = current->next;
    }
    current->next = malloc(sizeof(struct node));
    current->next->i = val;
    current->next->next = NULL;          // terminate list
  }
}

I thought an uninitialized pointer is same as NULL pointer in c, this helped me clear my doubt. — neeraj_nigam, Aug 01 '16 at 21:30
@neeraj_nigam There is also an option to zero initialize each node with `calloc`. See [here](http://en.cppreference.com/w/c/memory/calloc) for more information. The calling syntax is a little different, but not too crazy. — callyalater, Aug 01 '16 at 22:06

score 2 · Answer 3 · answered Aug 01 '16 at 21:06

The main reason why you do not see the effects of adding the second and subsequent nodes is that when you allocate a new node that is not the head node, you store the pointer to it only in local variable current of function add(). You need to instead store it in the next pointer of the previous node. Compare this with your original code:

    struct node* current = head;
    while (current->next != NULL) {
      current = current->next;
    }
    current->next = malloc(sizeof(*current->next));

Note also that, as @Lousy observed also, you fail to set the next pointers of new nodes to NULL. You must do this, for until you assign some value to those fields, their contents are indeterminate.

unable to understand Null pointer in c

3 Answers3