1

say I have the following code for inserting a node at the end to a linkedlist:

int main() {
    struct Node* head = NULL;
    newNode(head, 1);
    newNode(head, 2);
    newNode(head, 3);
    print(head);
    return 0;
}
void newNode(struct Node* head, int val) {
    struct Node* curr_p = head;
    struct Node* new_p = malloc(sizeof(struct Node));
    new_p->data = val;
    new_p->next = NULL;
    if (head == NULL) {
        curr_p = new_p;
        // printf("head %d \n", head->data); 
    }
    else{
        while (curr_p->next != NULL) {
            curr_p = curr_p->next;
        }
        curr_p->next = new_p;
    }
}
void print(struct Node* head) {
    struct Node* curr_p = head;
    while (curr_p != NULL) {
        printf("%d\n", curr_p->data);
        curr_p = curr_p->next;
    }
}

It appears what causes the error is in the if statement block where head == NULL, the head node pointer seems to unable to point to new node. I always ended up with a segmentation fault. any reason for this?

2 Answers2

0

You have not specified that where you are getting segmentation fault but my strong guess is that you would be getting at - printf("head %d \n", head->data);. Why? Because with your present code head is NULL (that's what you are passing from your main method) and then you try to de-reference a NULL then you WILL segmentation fault.

You must only de-reference a valid pointer. If the pointer variable is holding NULL or some uninitialized/default value then de-referencing it will result in segmentation fault.

Below is the fixed IF block.

if (head == NULL) {
    head = new_p;
    printf("head %d \n", head->data); 
} 
hagrawal7777
  • 14,103
  • 5
  • 40
  • 70
  • With this line `struct Node* curr_p = head;` you are assigning NULL to `curr_p`. Then you were really never setting the `head` so with each invocation of `newNode`, both `head` and `curr_p` were always NULL. You would have got the segmentation fault on first invocation of `newNode`because of the reason I outlined above (*put some `printf` and you can confirm the same*). With the code change I am suggesting, basically you are setting the `head`, this will not only prevent segmentation fault, but will ensure that node is appended at the last of the list, on subsequent invocation of `newNode` .. – hagrawal7777 Feb 21 '16 at 00:18
  • So, any news? Does it answer your queries? – hagrawal7777 Feb 21 '16 at 18:55
0

I guess that your fault is that you are passing the head pointer in value not in reference so the code doesn't change it's value because it just copies the pointer so your code should look like this

newNode(struct Node** head , int val ) {
struct Node* curr_p = *head;
/*

*/
if(curr_p == NULL ) {
*head = new_p;

}/*....*/
    }

and in the main

newNode(&head,1)

and the finale code would be

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

struct Node{
    int data;
    struct Node * next;
};
    void newNode(struct Node** head, int val) {

        struct Node* curr_p = *head;
        struct Node* new_p =malloc(sizeof(struct Node));
    new_p->data = val;
    new_p->next = NULL;
    if ( curr_p == NULL) {
        *head = new_p;
        // printf("head %d \n", head->data);
    }
    else{
        while (curr_p->next != NULL) {
            curr_p = curr_p->next;
        }
        curr_p->next = new_p;
    }
}
void print(struct Node* head) {
    struct Node* curr_p = head;
    while (curr_p != NULL) {
        printf("%d\n", curr_p->data);
        curr_p = curr_p->next;
    }
}
int main() {
    struct Node* head = NULL;
    newNode(&head, 1);
    newNode(&head, 2);
    newNode(&head, 3);
    print(head);
    return 0;
}

and actually you don't have to use curr_p in the print function because the head pointer in the print function it's just a copy of the head pointer of the main function so it wouldn't change it's value if you do something like head = head->next ; in the print function .

Rami ZK
  • 510
  • 3
  • 13