I am trying to insert value in linked list but in output it is showing only first insertion

Viewed 25

In this I am trying to insert values in empty linked list initially and then adding element after that . function insert is inserting element in linked list . display function is displaying linked list . so i am getting output as first insertion only .

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

struct node 
{
    int value;
    struct node *next;
}*first = NULL;

void insert(struct node *ptr,int n ){
    struct node* t;
    t=(struct node* )malloc(sizeof(struct node ));
    t->value=n;
    
    
    
    if(first==NULL){
        t->next=first;
        first=t;
        return;
    }
    else{
        ptr=first;
        while(ptr!=NULL){
            
            ptr=ptr->next;
        }
        
        t->next=ptr;
        t->value=n;
        ptr=t;
     }
    

}

void display(struct node *f){
    
    while(f!=NULL){
        printf("%d",f->value);
        f=f->next;
    
        }
      
        
}


int main(){
    
    insert(first,5);
    insert(first,20);
    insert(first,32);
    insert(first,66);
    insert(first,689);

    display(first);


    return 0;
}
1 Answers

First off, you're reassigning ptr before making use of its original value, and you're accessing start in the function directly anyway, so passing it as a function parameter is not needed.

Look more carefully at your appending code:

ptr=first;
while(ptr!=NULL){
    ptr=ptr->next;
}

// you only reach here when ptr is NULL!
    
t->next=ptr; // sets next to NULL
t->value=n; // you've already set the value after you malloc'd the node
ptr=t; // this line has no overall affect. You're setting the local pointer "ptr" to point to something then it goes out of scope.

In short, you've got your pointers the wrong way round. What you instead want to be doing here is getting up to the last element and then setting its next to the new node:

struct node* ptr=first;
while(ptr->next!=NULL){
    ptr=ptr->next;
}

// Now when we reach here, "ptr" is the last element in the list
ptr->next = t;

However, looping the entire list for every iteration is fairly inefficient, you could instead store a node* last as well as the first to avoid the loop:

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

struct node 
{
    int value;
    struct node *next;
} *first = NULL, *last = NULL;

void insert(int n){
    struct node* t;
    t = (struct node*)malloc(sizeof(struct node));
    t->value = n;
    t->next = NULL

    if (first == NULL) { // last == NULL too
        first = last = t;
        return;
    }
    
    last->next = t;
    last = t; 
}
Related