-4

The below code is giving an error, I don't have any idea why. The below function is to insert the node at the end of the linked list.

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

Node* Insert(struct Node *head,int data)
{ 
    struct Node *p;
    p=head;
    struct Node *prev;
    struct Node *temp=(struct Node*)malloc(sizeof(struct Node));
    temp->data=data;
    temp->next=NULL;

    if(head==NULL){
        head=temp;
        return head;
        //return temp;  
    }                                           ``
    while(p!=NULL){
        prev=p;
        p=p->next;
    }
    p=temp;//If we change this line to prev->next=temp .It is giving the correct result 
    return head;
}

In the above code if we replace the line (p=temp;) with prev->next=temp it works. Please help me to understand the logic behind this . Thanks in advance. Question is from hackrank.

MC93
  • 791
  • 6
  • 14
Vikas
  • 41
  • 5
  • 2
    Try to think: what will happen with `p` after the assignment? – qrdl Aug 29 '15 at 09:04
  • 1
    If two pointers point to the same memory location, changing one does not change the other. Both pointers are fully separated. If you want to change the other pointer, you can also use a pointer to a pointer. But that will make it much more complicated. – wimh Aug 29 '15 at 09:24
  • [Do not cast the return value of `malloc`.](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) – MC93 Aug 29 '15 at 09:59
  • `Node* Insert(struct Node *head,int data)` This is a syntax error in C (a struct definition is **not a typedef**) – wildplasser Aug 29 '15 at 11:03

1 Answers1

0

You assign the newly created node to p.That part is all right. However the previous node doesnt know the address of this newly created last element as prev->next is assigned to NULL. So you have to assign the previous next pointer to temp so that the node temp is Linked to your List.Just before the while loop ends p has some value but the next pointer is assigned to NULL. So next time prev has some data and the next pointer is NULL. You are not changing this next pointer to point to the next Node that is temp. Only assigning the temp to the p. Therefore the last temp is not linked to the list.

After the loop try :

 p=temp;
 prev->next=p;

Or just 

prev->next=temp;
Sourav Kanta
  • 2,727
  • 1
  • 18
  • 29
  • what do you mean ? can 't we assign the node after reaching the NULL pointer?? – Vikas Aug 29 '15 at 10:13
  • Yes you can and you are not doing it. The resulting 2nd last element has next pointer assigned to NULL and not the temp node. – Sourav Kanta Aug 29 '15 at 10:17
  • I am changing the value at p from NULL to the address of the temp where temp->next =NULL. Am i correct according to my code? – Vikas Aug 29 '15 at 10:30