CODEWITHSUNDEEP
csegmentation-fault
user1527166
user1527166

Reputation: 3279

Segfault in my Linked List implementation

Here's my code:

#include <stdio.h>

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

void push(node *top, int data) {
    node *new_node = (node*) malloc(sizeof(node));
    new_node->data = data;
    new_node->next = top;
    top = new_node;
}

int main() {
    node *top = (node*) malloc(sizeof(node));
    top->data = 1;
    printf("Set data of top node to: %d\n", top->data);

    push(top, 2);
    printf("Pushed 2 to top, top->next->data = %d\n", top->next->data);
}

The program segfaults at the 3rd last line (push(top, 2);) and I think on the line top = new_node;

I'm just learning C (pointers right now).

What did I do wrong?

Upvotes: 1

Views: 85

Answers (2)

P.P
P.P

Reputation: 121407

Pointer is passed by value to push. Hence, the change you did to top is not reflected in main. If you want to change top then pass the address of pointer:

#include <stdio.h>

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

void push(node **top, int data) {
    node *new_node = (node*) malloc(sizeof(node));
    new_node->data = data;
    new_node->next = *top;
    *top = new_node;
}

int main() {
    node *top = (node*) malloc(sizeof(node));
    top->data = 1;
    printf("Set data of top node to: %d\n", top->data);

    push(&top, 2);
    printf("Pushed 2 to top, top->next->data = %d\n", top->next->data);
}

Here's the relevant C-FAQ.

Upvotes: 1

Some programmer dude
Some programmer dude

Reputation: 409356

The problem here is that you pass the pointer to the top element by-value, and then you try to set the pointer inside the function but there it's just a local variable and changes to it will not be visible outside of the function.

Pass the top pointer by reference instead, by using a pointer to the pointer:

void push(node **top, int data) {
    node *new_node = malloc(sizeof(node));
    new_node->data = data;
    new_node->next = *top;
    *top = new_node;
}

...

push(&top, 2);

An alternative is to return the new top from the function instead:

node *push(node *top, int data) {
    node *new_node = malloc(sizeof(node));
    new_node->data = data;
    new_node->next = top;
    return new_node;
}

...

top = push(top, 2);

Upvotes: 5

Related Questions