Segmentation Fault in Linked List C program

Question

I am trying to create a list of 10 Nodes and assigning with the values 1 to 10 and printing them. I tried it with the following code, but I am ending up with segmentation fault.

I am very new to Linked Lists in C.

#include<stdio.h>
typedef struct Node
{
  int data;
  struct Node *next;
}Node_Struct;

int main(void)
{
 int i =0;
 Node_Struct* Node = NULL;
 Node = (Node_Struct*)malloc(sizeof(Node_Struct));

 for (i = 1; i<=10; i++){
    Node->data = i;
    Node       = Node->next;
 }

  for (i = 1; i<=10; i++){
    printf("\n Node->data:%d",Node->data);
    Node = Node->next;
  }
 return 0;
}

Answer 1

As pointed by people in comments, you're only allocating memory to head node only. You need to allocate memory for each node you're trying to add int that for loop also. Moreover you're moving on the Node pointer forward at each iteration, so you won't be able to traverse list after insertion. Keep a track of both head and tail of the list. Do the following:

Maintain head and tail of linked list:

 Node_Struct* headNode = NULL, *tailNode = NULL;
 // head node
 headNode = tailNode = (Node_Struct*)malloc(sizeof(Node_Struct));

Allocate memory at each iteration in the loop. It's your wish whether you want to keep something in head node or not. So change the code in for loop like this:

for (i = 1; i<=10; i++) {
   Node_Struct* newNode = (Node_Struct *)malloc(sizeof(Node_Struct));

   newNode->data = i;
   newNode->next = NULL;

   tailNode->next = newNode;
   tailNode = newNode;
}

After this you can iterate your list by copying head value in some other variable:

  Node_Struct *tmpNode = headNode;
  for (i = 1; i<=10; i++){
    printf("\n Node->data:%d",tmpNode->data);
    tmpNode = tmpNode->next;
  }

Answer 2

You are not allocating memory for each added node.

If to use your loops then it is enough to make these minor changes

Node_Struct* Node = NULL;
Node_Struct **current = &Node;

for (i = 1; i <= 10; i++ ) {
    *current = malloc(sizeof(Node_Struct));
    (*current)->data = i;
    (*current)->next = NULL;
    current = &(*current)->next;
}

current = &Node;
for (i = 1; i <= 10; i++) {
    printf("\n Node->data:%d", ( *current )->data);
    current = &( *current )->next;
}

Take into account that you should free all allocated memory for the nodes before exiting the program.

Answer 3

You have allocated space for just a single node, yet you are trying to loop over a list of linked nodes by doing this:

for (i = 1; i<=10; i++){
    Node->data = i;
    Node       = Node->next;
 }

When you do this - Node = Node->next;, Node might point to anywhere. You might be pointing to memory you are not supposed to touch.

One thing to remember is never to let go of the handle to the head of the list. Keep a pointer to the head of the list by maintaining a copy.

Node = malloc(sizeof(Node_Struct));
Node_Struct *head = Node;

Allocate space for each node by using malloc(). Build up the list of nodes first. One way to do that is like this:

for (i = 1; i<=10; i++){
    Node->next = malloc(sizeof (Node_Struct));
    Node       = Node->next;
}
Node->next = NULL    // The last node should point to NULL

After that you could set all the data field of the nodes in another loop or, set them in the same loop while doing malloc(). As you already have the handle to the head of the list, you know where to start. Like so:

Node = head;
i = 0;
while (Node) {
    Node->data = i++;
    Node = Node->next;
}

Answer 4

First I am showing you where your mistakes are and below I have re-written you code to fix your problem. See my codes and compare with yours. Hope that will help you to learn data structure.

Let’s see you mistakes

#include<stdio.h>
typedef struct Node
{
  int data;
  struct Node *next;
}Node_Struct;

int main(void)
{
  int i =0;
  Node_Struct* Node = NULL;
  Node = (Node_Struct*)malloc(sizeof(Node_Struct));

 for (i = 1; i<=10; i++){
     Node->data = i;/* till here everything fine */
     Node       = Node->next; /* now you are pointing to nowhere */
     /* in the next iteration of for loop Node->data = i will crash the application */
   /* you could have done it like below
     Node->next = (Node_Struct*)malloc(sizeof(Node_Struct));
     Node       = Node->next;
     Node->data = i;
     Node->next = NULL
     but here you lost the address of first node so all gone*/

 }

     for (i = 1; i<=10; i++){
     printf("\n Node->data:%d",Node->data);
     Node = Node->next;
  }
  return 0;
}

Now see my code below

int main(void)
{
    int i =0;
    Node_Struct *Node = NULL;
    Node_Struct *p = NULL;

    for (i = 1; i<=10; i++){
    if ( Node == NULL )
    {
         Node = (Node_Struct*)malloc(sizeof(Node_Struct));
         Node->data = i;
         Node->next = NULL;
         p = Node; /* p is pointing to the first Node of the list */
       }
       else
       {
           p->next = (Node_Struct*)malloc(sizeof(Node_Struct));
           p = p->next;
           p->data = i;
           p->next = NULL;
        }
  }

        p = Node; /* now p is pointing to first node of the link list */
         /* if you see above we always assign NULL to 'next' pointer so that the last node of the list    pointing to NULL */
        /* Therefore in the below while loop we are searching the list untill we reach the last node */
        while( p != NULL )
        {
            printf("\n p->data:%d",p->data);
            p = p->next;
         }
      return 0;
  }