Singly Linked list basic

Question

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;
}

Answer 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.