0

I had written the code just to create a linked list but it's showing segmentation fault. Can anybody tell me what's the mistake and what should I do to resolve it

My code for the singly linked list

// defining the linked list (basics)
#include <stdio.h>
#include <stdlib.h>

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

void create(struct node **head, int data)
{
   struct node *temp;

if (*head == NULL)
{
    *head = (struct node*)malloc(sizeof(struct node));
    (*head) -> data = data;
    (*head) -> next = NULL;
}
else
{
    temp = *head;
    while (temp != NULL)
    {
        temp = temp -> next;
    }
    temp = (struct node*)malloc(sizeof(struct node));
    temp -> data = data;
    temp -> next = NULL;
}
}

int main()
{
struct node *begin = NULL;
create(&begin, 1);
create(&begin, 7);
printf("%d\n", begin -> next -> data);
printf("%lu\n", sizeof(struct node));

return 0;
}
1

You assign to temp, which is a local variable, the node never ends up in the linked list.

I'd use pointers to pointers all the time, for example like

void create(struct node **head, int data)
{
    struct node **temp = head;
    while (*temp != NULL)
    {
        temp = &(*temp) -> next;
    }
    (*temp) = (struct node*)malloc(sizeof(struct node));
    (*temp) -> data = data;
    (*temp) -> next = NULL;
}

An alternative version more closely resembling your code, is

void create(struct node **head, int data)
{
    if (*head == NULL)
    {
        (*head) = (struct node*)malloc(sizeof(struct node));
        (*head) -> data = data;
        (*head) -> next = NULL;
    }
    else
    {
        struct node *temp = *head;
        while (temp -> next != NULL)
        {
            temp = temp -> next;
        }
        temp -> next = (struct node*)malloc(sizeof(struct node));
        temp -> next -> data = data;
        temp -> next -> next = NULL;
    }
}

Just a thought, adding elements at the beginning of the list is much easier:

void create(struct node **head, int data)
{
    struct node *temp = (struct node*)malloc(sizeof(struct node));
    temp -> data = data;
    temp -> next = *head;
    *head = temp;
}

If you want to do it right, you'd also test malloc's return value for being NULL, at least in production code.

| improve this answer | |
  • Is there any mistake if I declare the struct node temp in the begining of the function assign it's value in the else part. Like: struct node *temp; and inside else block *temp = *head; I am telling this about 2nd function which you have suggested. – Rahul Oct 27 '17 at 17:46
  • Code in comments is almost unreadable. Put backticks ` around to create inline code blocks, this preserves the * that would otherwise be taken for markdown. – Blauelf Oct 27 '17 at 17:48
  • As long as you don't use a variable's value before assigning to it, it doesn't really matter where you declare and where you do the initial assignment. Order should be declare->assign->read. – Blauelf Oct 27 '17 at 17:49
  • Actually it's showing error something like this link.c:22:15: error: assigning to 'struct node' from incompatible type 'struct node *'; dereference with * *temp = *head; ^ ~~~~~ 1 error generated. make: *** [link] Error 1 – Rahul Oct 27 '17 at 17:50
  • You mean you used struct node *temp; and temp = *head;? – Blauelf Oct 27 '17 at 17:52

You must log in to answer this question.

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