0

I'm attempting to create a linked list in C to practice the concepts of pointers and data structures. I keep running into a segmentation fault and I've narrowed down the issue to my insertNode() function.

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

struct node {
    char *value;
    struct node *next;
};
typedef struct node node_t;

node_t* getNode(char *val) {
    node_t *node = malloc(sizeof(node_t));
     
    if (node == NULL) return node_t;
    
    node->value = val;
    node->next = NULL;
    return node;
}

void printList(node_t *node) {
    printf("%s\n", node->value);
    
    if (node->next == NULL) return;
    
    printList(node->next);
}

void insertNode(node_t *head, char* val) {
    node_t *current = head;
    
    while (current!= NULL) {
        current = current->next;
    }
    
    node_t *newNode = getNode(val);
    current->next = newNode;
}

int main() {
    node_t *head = getNode("Alpha");
    
    insertNode(head, "Bravo");

    printList(head);
    
    return 0;
}

The issue is specifically with condition in the while loop. As far as I understand, current should initially equal head which points to the first node in the linked list, so it should enter the loop and set current to current->next which is a NULL pointer. On the next iteration it will therefore not enter the loop, and then point the NULL pointer to the newly created node instead.

void insertNode(node_t *head, char* val) {
    node_t *current = head;
    
    while (current != NULL) {
        current = current->next;
    }
    
    node_t *newNode = getNode(val);
    current->next = newNode;
}

Changing my while loop logic to the below seems to resolve the segmentation fault and the program works as expected but I don't understand why the previous condition was causing an issue.

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

Does this not just skip a step and jump to the first next pointer, I'm failing to understand why the previous while loop causes a segmentation fault and doesn't achieve the same behaviour.

1

In your insertNode function, while your iterating to find the last node, you should check whether the next_node is NULL, not current_node is NULL.

Let's say, we had two nodes

  1   ->   2   ->   NULL
head

When you start iterating, using

while(current != NULL) 
   current = current -> next;

It will eventually reach the node 2. Now this is the last node, you should stop here, but according to your while loop(current != NULL), it will move onto NULL and then break out of the loop.

So current = NULL at the moment. When you do current -> next = newNode, you're de-referencing NULL, resulting in seg-fault.

So only modification would be

while(current -> next != NULL) //Fetch me the last node

But there is till one important thing to do. You're not checking whether list is empty or not inside insertNode function.

if (head == NULL) {
    node_t *newNode = getNode(val);
    head = newNode;
    return;
}

Edit: Looks like you're assigning a node to head inside main function. The above check is not necessary

5
  • Thank you very much! The diagram makes it much easier to visualise the pointer trail. One further question I had is around passing by reference vs passing by value, is there a reason that I don't have to pass a pointer to a pointer which would make it insertNode(node_t **head, char *val) whereas when using an array of pointers you do have to do this? – docnocturnal Apr 14 at 16:04
  • Sorry I think my last comment was slightly confused, what I was trying to ask is that I believe a copy of the head pointer is created when passed to the insertNode() function. If I tried setting this copy equal to another memory address, this would not reflect on the original pointer passed into the function I assume? Is that why my insertNode() function is okay to accept a pointer rather than a pointer to a pointer because I am simply dereferencing and not trying to overwrite the memory address stored within the pointer? – docnocturnal Apr 14 at 16:15
  • @docnocturnal Wrong, when you're giving head as an argument you're passing the reference, not the value. Essentially you're sending the actual address not a copy of address(doesn't make sense in C) – C-- Apr 14 at 17:18
  • But if you were to create a function that say swapped the memory addresses of two pointers, you would have to pass a pointer to each pointer since the function would create a local variable that is a copy of each pointer, and thus would not reflect on the pointers passed in from the main function (similar to the swap.c example in Week 4) right? I'm not sure if I'm phrasing this correctly – docnocturnal Apr 14 at 18:39
  • @docnocturnal Pass-by-value vs Pass-by-reference. One of them uses copies, other gets the address, goes to address and manipulates values inside the address – C-- Apr 14 at 18:52

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .