Why current = current->next gives segmentation fault when current->next == NULL?

Question

I have below C program to implement an ascending order linked-list. Problem is in buildList() function so you can ignore other function apart from main() and buildList().

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

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

void buildList(struct node **, int);
void printList(struct node **);

int main()
{
    struct node *head;
    head = NULL; /*Empty list*/

    int hold, i, j;
    printf("How many integers in this list: ");
    scanf("%d",&i);
    j = 1;
    for(i; i > 0; i--)
    {
            printf("Integer %d: ",j);
            scanf("%d",&hold);
            buildList(&head, hold);
            j++;
    }
    printList(&head);
    return 0;
}

void buildList(struct node **headRef, int data)
{
    struct node *newNode;
    newNode = malloc(sizeof(struct node));
    struct node *current, *current1;
    current = *headRef;
    //If list is empty, add the number as the first node.
    if (current == NULL)
    {
            newNode->data = data;
            newNode->next = *headRef;
            *headRef = newNode;
    }
   //If the list is not empty.
    else
    {
            //If the number is not greater than first number.
            if (data <= current->data)
            {
                    newNode->data = data;
                    newNode->next = current;
                    *headRef = newNode;
            }
            //If the number is greater than the first number in list.
            else
            {
                    int flag = 0;
                    while (data > (current->data))
                    {
                            current1 = current;
                            current = current->next;
                    }


                    newNode->data = data;
                    current1->next = newNode;
                    newNode->next = current;
            }
    }
}

//Prints nodes and total number of nodes.
void printList(struct node **headRef)
{
    int count = 0;
    struct node *current;
    current = *headRef;
    while (current != NULL)
    {
            count++;
            printf("%d ",current->data);
            current = current->next;

    }
    printf("\nTotal nodes: %d\n",count);
}

This program works fine untill I give a number which is bigger than any number present in the list. In that case I get a segmentation fault.

Case I (Fine correct output)

-bash-4.1$ ./a.out
How many integers in this list: 3
Integer 1: 5
Integer 2: 1
Integer 3: 4
1 4 5
Total nodes: 3
-bash-4.1$

Case II (Here code breaks(segmentation fault))

-bash-4.1$ ./a.out
How many integers in this list: 3
Integer 1: 5
Integer 2: 6
Segmentation fault
-bash-4.1$

After a long time trying to figure out what was going wrong in my code, I figured out that when a number was bigger then any number in the list, in which case it was to be inserted at the end of the list, this statement (in the last else in function buildList()) was causing the issue:

  current = current->next;

I finally figured out that when current->next is NULL, this statement cause seg fault.

I came up with a workaround like below and it worked giving correct output.

 else
 {
     int flag = 0;
     while (data > (current->data))
     {
         current1 = current;
         if(current->next != NULL)
         {
             current = current->next;
         }
         else
         {
             flag = 1;
             break;
         }
    }
    newNode->data = data;
    current1->next = newNode;
    if (flag == 1)
    {
       newNode->next = NULL;
    }
    else
    {
       newNode->next = current;
    }
}

Now I get correct output.

-bash-4.1$ ./a.out
How many integers in this list: 3
Integer 1: 5
Integer 2: 1
Integer 3: 10
1 5 10
Total nodes: 3
-bash-4.1$

Now I am left wondering why wasn't current = current->next; working when current->next was NULL. I was expecting this statement to assign NULL to current.

Could anyone please tell me the reason for this ? Also was my workaround a good one ? Or is there a better way to do it?

Sorry for this long question, but spending like 2 hours debugging this, i guess I am going crazy.

Thanks.

Answer 1

This while loop will not stop when the second input is greater than the first input.

     while ( data > (current->data))
     {
        current1 = current;
        current = current->next;
     }

You end up dereferencing a NULL pointer. You need:

     while ( current != NULL && data > (current->data))
     {
        current1 = current;
        current = current->next;
     }

Answer 2

Your debugging technique is giving you a bit of a red herring. The problem is in this loop:

while (data > (current->data))
{
    current1 = current;
    current = current->next;
}

But not with the line you think it is. If current->next is NULL, current gets set to NULL just as you describe. The next operation that takes place, however is the loop condition - which dereferences current. Blammo. You probably want something like:

while (current && (data > current->data))
{
    current1 = current;
    current = current->next;
}

I didn't analyze your whole program, so I can't really comment on the correctness of this part of it other than the obvious crasher.