CODEWITHSUNDEEP
cpointerslinked-list
Ambareesh S J
Ambareesh S J

Reputation: 97

Why is this linked list printing the last element indefinitely?

I was completing a Hackerrank challenge involving addition of elements to a linked list and printing it.

The input is of this format: A set of integers, in which the first element gives the size and the rest are the components of the list.

I completed the challenge in Java, but I am not able to do it in C. Input of 4 2 3 4 1 should print 2 3 4 1, but this snippet that I coded is giving me 1 1 1 1 1 1 .... {truncated}

My approach: Declare a new struct temp of type Node (with the data input as data field, and NULL in next field), then iterate thorough the linked list with head as the starting point, and when it reaches the last element, change the next field of the last element to the address of the current element.

Code:

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

   typedef struct Node{
   int data;
   struct Node* next;
    }Node;

 Node* insert(Node *head,int data)
  {       
    Node temp = {data, NULL} ;

     if (head == NULL)
      { head =&temp;
        return head;
      }

     else{

    Node* current = head ;
    while(current->next !=NULL)
      current = current->next ;

      current->next = &temp;

      return head;
       }
   }

void display(Node *head)
{
Node *start=head;
while(start)
{
    printf("%d ",start->data);
    start=start->next;
}
}

int main()
{
int T,data;
scanf("%d",&T);
Node *head=NULL;    
while(T-->0){
    scanf("%d",&data);
    head=insert(head,data);
            }

 display(head);

 }

Upvotes: 1

Views: 401

Answers (2)

Some programmer dude
Some programmer dude

Reputation: 409482

Because you save a pointer to a local variable in the list. When the insert function returns, the local variable temp will go out of scope and cease to exist. Saving a pointer to it and using that pointer will lead to undefined behavior. Find a good beginners book and read about dynamic allocation of memory.

Upvotes: 2

Sergey Kalinichenko
Sergey Kalinichenko

Reputation: 727087

List nodes must be allocated dynamically. This

Node temp = {data, NULL} ;

declares a local variable. Referring to its address outside the scope of its declaring function is undefined behavior.

Replace with

Node *temp = malloc(sizeof(Node));
temp->data = data;
temp->next = NULL;

Now thtat temp is a pointer, expression &temp has to be replaced with temp as well.

Upvotes: 5

Related Questions