Linked List error "Segmentation Fault" Core Dumped

Question

Trying on Fedora gcc the following code for a simple Linked List adding new node to the tail of the list. No error in compilation. During execution, it is showing Segmentation Fault, Core Dumped. On MS Windows, it is working.

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

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

void insertion(struct Node *);
void display(struct Node *);

int main(void)
{
    struct Node *head;
    head=NULL;
    head->next=NULL;

    int choice, cont;

    do
    {
        printf("1.Insert      2.Display       3.Exit");
        scanf("%d",&choice);

        if(choice==1)
        {
            insertion(head);
        }
        else if(choice==2)
        {
            display(head);
        }
        else if(choice==3)
        {
            exit(0);
        }
        else
        {
            printf("Wrong choice");
        }
        printf("Continue? Press 1 otherwise 0:");
        scanf("%d",&cont);
    }while(cont==1);

    return 0;
}

void insertion(struct Node *start)
{
    int data;
    struct Node *temp=NULL;
    temp->next=NULL;
    struct Node *mnew=NULL;
    mnew->next=NULL;

    mnew=(struct Node *)malloc(sizeof(struct Node));

    printf("Enter data:");
    scanf("%d",&data);

    mnew->data=data;

    if(start==NULL)
    {
        start=mnew;
    }
    else if(start!=NULL && start->next==NULL)
    {
        start->next=mnew;
    }
    else
    {
        temp=start;
        while(temp->next!=NULL)
        {
            temp=temp->next;
        }
        temp->next=mnew;
    }
}

void display(struct Node *start)
{
    struct Node *temp=NULL;
    temp->next=NULL;    
    if(start==NULL)
    {
        printf("\nNothing to display!");
    }
    else if(start!=NULL && start->next==NULL)
    {
        printf("%d",start->data);
    }
    else
    {
        temp=start;
        while(temp!=NULL)
        {
            printf("%d",temp->data);
            temp=temp->next;
        }
    }
}

Your help is appreciated.

Moreover start=mnew; does not change head main value..... You should study something about pointers. — LPs
– LPs, Commented Feb 28, 2017 at 11:09

Chris Turner · Accepted Answer · 2017-02-28 11:04:15Z

5

head=NULL;
head->next=NULL;

This piece of code could never work as you cannot access or assign values to attributes of head if it's pointing to NULL (aka nowhere).

answered Feb 28, 2017 at 11:04

Chris Turner

8,1421 gold badge16 silver badges18 bronze badges

Sign up to request clarification or add additional context in comments.

6 Comments

PS Nayak Over a year ago

I have commented those second lines and now it is executing. But after inserting 3 nodes, the display function is reporting "Nothing to display!" that means the start is still NULL.

Chris Turner Over a year ago

That is because you're not updating head - you either have to pass it into your insertion function as a reference or have that function return the new start so you can assign it to head

LPs Over a year ago

Moreover start=mnew; does not change head main value

PS Nayak Over a year ago

Ok, I passed the address and it is working now. Thanks to all of you.

Chris Turner Over a year ago

@LPs not sure changing display is required since it doesn't modify the list

|

Some programmer dude · Accepted Answer · 2017-02-28 11:02:47Z

2

Take a closer look at e.g. these two lines from the insertion function:

struct Node *temp=NULL;
temp->next=NULL;

The first defines a pointer to struct Node and makes it be a null pointer. The very next line you dereference this null pointer, which is invalid and leads to undefined behavior.

You have the same problem in multiple places, both exactly like this, and also dereferencing null pointers in general.

answered Feb 28, 2017 at 11:02

Some programmer dude

411k36 gold badges420 silver badges655 bronze badges

8 Comments

LPs Over a year ago

... or OP can use calloc instead of malloc

LPs Over a year ago

Moreover start=mnew; does not change head main value

StoryTeller - Unslander Monica Over a year ago

@LPs - If you are suggesting calloc as a shortcut to set all pointers in the structure to NULL, know that a zero bit pattern is not guaranteed to be the value of a NULL pointer. See this standard note on calloc for reference.

BLUEPIXY Over a year ago

@LPs All bits 0 is not necessarily the same as NULL.

LPs Over a year ago

@BLUEPIXY Well, sure but not the case here, I guess

|

Vlad from Moscow · Accepted Answer · 2017-02-28 11:27:07Z

1

You may not access data using a null pointer. Thus this code snippet (and similar code snippets)

struct Node *head;
head=NULL;
head->next=NULL;
^^^^^^^^^^^^^^^

is invalid.

As for the function insertion then you have to pass the head by reference. Otherwise the function will deal with a copy of the head and any changes of the copy of the head in the function will not influence on the original head.

Also if the memory allocation will failure it is desirable that the function signals about this. So instead of the return type void it is better to use the return type int.

So the function declaration can look like

int insertion( struct Node ** );
^^^            ^^^^^^^^^^^^^^

The function can be defined like

int insertion( struct Node **start )
{
    int data;

    printf( "Enter data: " );
    scanf( "%d", &data );

    struct Node *temp = ( struct Node * )malloc( sizeof( struct Node ) );

    int success = temp != NULL;

    if ( success )
    {
        temp->data = data;
        temp->next = NULL;

        while ( *start ) start = &( *start )->next;

        *start = temp;
    }

    return success;
}

The function can be called the following way

insertion( &head );

The function display can look like

void display( struct Node *start )
{
    if ( start == NULL )
    {
        printf( "\nNothing to display!\n" );
    }
    else 
    {
        for ( ; start; start = start->next )
        {
            printf( "%d ", start->data );
        }
        putchar( '\n' );
    }
}

edited Feb 28, 2017 at 11:27

answered Feb 28, 2017 at 11:18

Vlad from Moscow

313k27 gold badges204 silver badges358 bronze badges

1 Comment

Ajay Brahmakshatriya Over a year ago

I am surprised as to why this is even working on Windows? Also any decent compiler should be able to catch this and present a warning.

conan tannous · Accepted Answer · 2017-08-06 16:13:54Z

0

As the previous comment, directing a NULL pointer to another NULL is not defined (cause a pointer should hold an address). Now some suggestions:

1) define the struct like so:

typedef struct Node *node_pointer;

this will make it easier to define pointers for that struct.

2)

mnew=malloc(sizeof(*mnew)); //this is easier, and this should be before the ..->next=NULL;

also check if allocation succeed:

if (!mnew)
  return; //return something needed

edited Aug 6, 2017 at 16:13

user6250760

answered Feb 28, 2017 at 11:21

conan tannous

991 gold badge1 silver badge7 bronze badges

Collectives™ on Stack Overflow

Linked List error "Segmentation Fault" Core Dumped

4 Answers 4

6 Comments

8 Comments

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

6 Comments

8 Comments

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related