CODEWITHSUNDEEP
cstructlinked-listsegmentation-fault
zaSam
zaSam

Reputation: 3

C passing a linked list struct to a function

I implemented a Linked List in C.

The problem is that whenever I try to print the nodes data using the function print_list(struct linked_list *list) I get a segmentation fault.

I am not sure what is causing it because, when I try print(struct linked_list *list), it works fine.

And, when I try allocating the memory dynamically it works fine also. But I am curious what is wrong with the code this way? And why using print won't lead to the same error?

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

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

struct linked_list{
    struct node *head;
};

void concat(struct linked_list* list1, struct linked_list* list2)
{
    struct node* tmp = list1->head;
    
    while(tmp->next != NULL)
        tmp = tmp->next;
    tmp->next = list2->head;
}

void print_list(struct linked_list *list)
{
    struct node* tmp = list->head;
    while(tmp != NULL){
        printf("%c - ", tmp->data);
    tmp = tmp->next;}
    printf("\n");
}

void print(struct linked_list *list)
{
    struct node* tmp = list->head;
    printf("%c\n", tmp->data);
    tmp = tmp->next;
    printf("%c\n", tmp->data);
    tmp = tmp->next;
    printf("%c\n", tmp->data); 
}

int main()
{
    struct linked_list list1,list2;
    struct node n1,n2,n3,n4,n5;
    n1.data = 'A';
    n2.data = 'B';
    n3.data = 'C';
    n4.data = 'D';
    n5.data = 'E';
    n1.next = &n2;
    n2.next = &n3;
    
    n4.next = &n5;
    list1.head = &n1;
    list2.head = &n4;
    printf("List 1 containes :\n");
    print_list(&list1);
    concat(&list1,&list2);
    printf("List 1 after concat: \n" );
    print_list(&list1);

    return 0;
}

Upvotes: 0

Views: 71

Answers (2)

hafiz031
hafiz031

Reputation: 2670

First try to understand what segmentation fault is.

According to Wikipedia:

In computing, a segmentation fault (often shortened to segfault) or access violation is a fault, or failure condition, raised by hardware with memory protection, notifying an operating system (OS) the software has attempted to access a restricted area of memory (a memory access violation).

So this is happening because your program is accessing to a restricted location where it is not meant to access. But why? Because in while(tmp->next != NULL) the program is not necessarily finding NULL when it is done traversing all the elements. Thus, in that case the loop doesn't break and it allows the loop to continue further and eventually the program tries to access to a restricted location.

So to combat this, initialize the node* Next = NULL in your node structure's definition. Like:

struct node{
    char data;
    struct node* next = NULL;
};

Now the default value for next is set to NULL explicitly. Hence, unless you change it to point to another node, it will still remain NULL. And the problem should be solved.

Upvotes: 0

M Oehm
M Oehm

Reputation: 29126

Here:

struct node n1,n2,n3,n4,n5;

you create five nodes without initializing them. C doesn't initialize local variables to null or zero, so the fields of nodes have indeterimate ("garbage") values. Later, you initialize some of the fields, but not the next fields of the last nodes in the list.

There are several solutions, for example:

(1) Initialize the next fields of the last nodes explicitly:

n1.next = &n2;
n2.next = &n3;
n3.next = NULL;

n4.next = &n5;
n5.next = NULL;

(2) Initialize the nodes with data, then set the links:

struct node n1 = {'A'};
struct node n2 = {'B'};
struct node n3 = {'C'};
struct node n4 = {'D'};
struct node n5 = {'E'};

n1.next = &n2;
n2.next = &n3;

n4.next = &n5;

Once you initialize a struct, all fields will be initialized, even if the values (like for next) are not given explicitly. Depending on the type, these fields are initialized implicitly with zero or null. Now you have valid (but unconnected) nodes begore you set the links.

(3) Define everything by initialization:

struct node n5 = {'E', NULL};
struct node n4 = {'D', &n5};
struct linked_list list2 = {&n4};

struct node n3 = {'C', NULL};
struct node n2 = {'B', &n3};
struct node n1 = {'A', &n2};
struct linked_list list1 = {&n1};

Now you have your list ready, but you must defined it backwards, so that the next node is known when you refer to it.

There are other possibilities to set up a linked list without allocating memory on the heap, for example an array of nodes, but I think you get the idea.

Upvotes: 1

Related Questions