C Linked List copy segfault

Question

I am rather new to C, and am working on copying a Linked List. It seg faults somewhere in the while loop, I think I am having some pointer troubles. Also, I'm not sure if I need to malloc each 'next' node. Do I? It makes sense for me to have to.

struct node* copyList() {
  struct node* walker = head;  // starting point to "walk" the list
  struct node* temp;
  temp = (struct node*)malloc(sizeof(struct node));
  temp->data = walker->data;

  while( walker != NULL ){ // done when we reach the node pointing to NULL
     walker = walker->next;    // advance walker to the next node in the list
     temp = temp->next;
     temp = (struct node*)malloc(sizeof(struct node));
     temp->data = walker->data;
     }
  return walker;
}

The node strut is as follows

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

Answer 1

Suppose you reach last node..

Now inside loop , you increment walker..so now walker = NULL..

so this statement gives an error temp->data = walker->data..

Also you are just creating nodes and copying data not connecting your new linked list..

1. You need to maintain new Head pointer to return at the end
2. Keep reference to previous node so that you can link it to current node
3. Update the pointers

Change it along the lines of this..

struct node* copyList() {
   struct node* walker = head;  // starting point to "walk" the list
   struct node* newHead=NULL,temp,prev=NULL;

   while( walker != NULL ){ // done when we reach the node pointing to NULL
     temp = (struct node*)malloc(sizeof(struct node)); //create new node
     temp->data = walker->data;          //copy data
     if(prev==NULL)                      //if its first node
         newHead = temp;                 //new head pointer                  
     else          
         prev->next = temp;              //else link to previous node                  
     prev = temp;                        //update pointers
     walker = walker->next;
   }
   return newHead;
}

Answer 2

Just where do you expect the value of temp->next in your loop to come from?

Also, to get a bit more meta, you might be vastly better off using e.g. std::list in C++ rather than implementing your own data structures like this. Even for experienced engineers, such efforts are notoriously error-prone.