Segmentation fault while creating a linked list

Question

I am writing a small program which stores data and key inside a linked list structure, and retrieves data based on a key from the user. The program also checks whether it is a unique key and if it so it stores the data by creating a node at the front of the list. But the below code throws segmentation fault all the time.

 #include<stdlib.h>

/* Node having data, unique key, and next */.  
struct node
{
    int data;
    int key;
    struct node *next;
}*list='\0',*p;

/* Create a node at the front */
void storeData(int data_x,int key_x)
{
    int check_key;
    position *nn;  //nn specifies newnode
    nn=(position)malloc(sizeof(struct node));

/* Segmentation Fault occurs here */
    if(list->next==NULL)
    {
        nn->next=list->next;
        nn->data = data_x;
        nn->key = key_x;
        list->next = nn;
    }
    else
    {
      check_key=checkUniqueKey(key_x);
      if(check_key != FALSE)
      {
         printf("The entered key is not unique");
      }
      else
      {
          nn->data = data_x;
          nn->key = key_x;
          nn->next=list->next;
          list->next=nn;
      }

   }
}

/* Retreive data based on a key */

int retreiveData(int key_find)
{
   int ret_data = NULL;
   p=list->next;
   while(p->next != NULL)   
   {
        if(p->key == key_find)
        {
            ret_data = p->data;
            break;
        }
     p=p->next;
   }  
   return(ret_data);
}
/*  Checks whether user key is unique */
int checkUniqueKey(int key_x)
{
    int key_check = FALSE;
    p=list->next;
    while(p->next != NULL)
    {
        if(p->key == key_x)
        {
          key_check = TRUE;
          break;    
        }
      p=p->next;
    }
    return(key_check);
}

The segmentation fault occurs in the storeData function after the dynamic allocation.

if(list->next==NULL) { nn->next=list->next; You are dereferencing a NULL pointer here. Also : *list='\0' is not a proper initializer for a pointer. — wildplasser
– wildplasser, Commented Feb 17, 2018 at 21:35
Using global variable like this is very bad; Don't assign '\0' to a pointer, don't assign NULL to a int; Code is incomplete, but I haven't found anywhere you malloc for list, list->next is probably deref a NULL because of this. — llllllllll
– llllllllll, Commented Feb 17, 2018 at 21:39

chqrlie · Accepted Answer · 2018-02-17 22:42:47Z

There are some problems in your code:

your list handling is flawed: you always dereference the global pointer list, even before any list items are created. You should instead test if the list is empty by comparing list to NULL.
type position is not defined. Avoid hiding pointers behind typedefs, this is a great cause of confusion, which explains your mishandling of list pointers.
avoid defining a global variable with the name p, which is unneeded anyway. Define p as a local variable in the functions that use it.
NULL is the null pointer, 0 a zero integer value and \0 the null byte at the end of a C string. All 3 evaluate to 0 but are not always interchangeable. For better portability and readability, use the appropriate one for each case.

Here is an improved version:

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

/* Node having data, unique key, and next */.  
struct node {
    int data;
    int key;
    struct node *next;
} *list;

/* Create a node at the front */
void storeData(int data_x, int key_x) {
    if (checkUniqueKey(key_x)) {
        printf("The entered key is not unique\n");
    } else {
        /* add a new node to the list */
        struct node *nn = malloc(sizeof(struct node));
        if (nn == NULL) {
            printf("Cannot allocate memory for node\n");
            return;
        }
        nn->data = data_x;
        nn->key = key_x;
        nn->next = list;
        list = nn;
    }
}

/* Retrieve data based on a key */
int retrieveData(int key_find) {
    struct node *p;
    int ret_data = 0;

    for (p = list; p != NULL; p = p->next) {
        if (p->key == key_find) {
            ret_data = p->data;
            break;
        }
    }
    return ret_data;
}

/* Checks whether user key is unique */
int checkUniqueKey(int key_x) {
    struct node *p;
    int key_check = FALSE;

    for (p = list; p != NULL; p = p->next) {
        if (p->key == key_x) {
            key_check = TRUE;
            break;  
        }
    }
    return key_check;
}

M. Gouin · Accepted Answer · 2018-02-17 21:41:59Z

0

You try to cast your address on a position structure instead of a position* nn=(position)malloc(sizeof(struct node)); Compile your code with gcc flags -Wextra and -Wall to prevent this kind of issue. Moreover I don't know is it is a mistake but malloc a size of struct node and your nn variable is a pointer on position.

answered Feb 17, 2018 at 21:41

M. Gouin

111 silver badge3 bronze badges

2 Comments

Coder Over a year ago

typedef struct node* postion; I have included this statement in my code

M. Gouin Over a year ago

Your nn variable is a postion *, so it is a struct node **, are you sure this is what you want to do ?

Strong will · Accepted Answer · 2018-02-17 21:50:33Z

0

When you initialized your list pointer you set it to NULL(as '\0'), when the program accesses address 0x00 it goes out of its boundaries and the operating system kills the process.

To avoid the segfault you can have "list" of non pointer type thus allocating on stack, when you want to access list as pointer you can do &list. Another solution would involve having variable on stack "root_node" and initialize list pointer as list = &root_node.

answered Feb 17, 2018 at 21:50

Strong will

5684 silver badges10 bronze badges

Collectives™ on Stack Overflow

Segmentation fault while creating a linked list

3 Answers 3

Comments

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related