CODEWITHSUNDEEP
cpointerslinked-list
Ravi
Ravi

Reputation: 303

linked list print issue

Why does my linked list print only last two elements instead of complete list? I have four nodes in the list, but print only the last two.

Expected: 10--->20--->30--->40

Actual: 30-->40

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

struct node *head=NULL;

void add(struct node **head, int d)
{
    struct node **p;
    p = head;
    struct node *t = malloc(sizeof(struct node));
    t -> data = d;
    t -> next = NULL;
        
    if(*p==NULL)
    {
        *p = t;
    }else{
        while((*p)->next !=NULL)
        {
            (*p) = (*p) -> next;
        }
        (*p) -> next = t;
    }
}   

void print(struct node *head)
{
   struct node *t;
   t = head;
   while(t!=NULL)
   {
       printf("%d", t->data);
       t = t -> next;
   }
}

main()
{
   add(&head,10);
   add(&head,20);
   add(&head,30);
   add(&head,40);
   print(head);
}

Upvotes: 0

Views: 49

Answers (2)

WhozCraig
WhozCraig

Reputation: 66194

This is wrong:

(*p) = (*p) -> next;

This says "set the value of the pointer addressed by the pointer-to-pointer p to the value of the node currently addressed by said-same's next member." This has all the appearances of cookie-cutting code without understanding what it does.

The proper form to do this, which removes the special case of testing for a null head as a bonus, should look like this:

void add(struct node **pp, int d)
{
    while (*pp)
        pp= &(*pp)->next;

    *pp= malloc( sizeof **pp);
    if (*pp)
    {
        (*pp)->data = d;
        (*pp)->next = NULL;
    }
    else
    {
        perror("Failed to expand linked list");
        exit(EXIT_FAILURE);
    }
}

The pp pointer-to-pointer in this function initially addresses the head pointer back in the caller. If the list is empty that pointer will be null, and thus *pp here will be null. Therefore the while-loop will self-terminate immediately. Otherwise it will walk the list, each time adjusting pp (not *pp) to hold the address of the next member of whatever node it currently refers. This effectively follows a chain of pointers. Remember, pp holds the address of a pointer to node, not the address of a node. When it lands on the last pointer in the list (the last next member in the last node) it will contain the address of that next, which is pointing to NULL. From there, the new node can be hung and we're done.

Upvotes: 1

1201ProgramAlarm
1201ProgramAlarm

Reputation: 32732

The problem isn't in the printing, it is in the adding.

You successfully add the first two nodes. When you add the third, (*p)->next is not NULL, so you execute the body of the while loop, (*p) = (*p)->next;. This will overwrite *head with, leaking the first node and reducing you list to having only one node (the second one you added). Then the new node is added after that node, resulting in the list again having two nodes. This continues when adding other nodes.

To fix this, you don't want to write to *p except in the case that the list is empty. In that case, though, you can use *head = t;. This allows changing the type of p from struct node ** to a single pointer, struct node *p. Initialize it with *head, then all the reads/writes to (*p) will just be p.

Upvotes: 0

Related Questions