Linked List insertion in Beginning

Question

I am trying basic creation of linked list using C. I have written the following code which is working up until first node but fails eventually on second one. I think the issue is where I am trying to display the node values in list separated by arrow(->). I think my logic is right but please correct me. Thanks in advance

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

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

typedef struct node NODE;
NODE *node1, *node2, *start, *save;

int main()
{
    node1 = (NODE *)malloc(sizeof(NODE));

    int i = 0;
    start = NULL;

    for(i = 0; i < 3; i++)
    {
        int inf;

        printf("Enter node value:");
        scanf("%d", &inf);

        node1->number = inf;
        node1->next = NULL;

        if(start == NULL)
        {
            start = node1;
            save = node1;
        }
        else
        {
            // save=start;
            // start=node1;
            // node1->next=save;
            node1->next = start;
            start = node1;
        }

        while(node1 != NULL)
        {
            printf("%d ->",node1->number);
            node1 = node1->next;
        }
   }

   return 0;
}

Answer 1

The issues are

• How you're allocating your nodes for insertion (i.e. save for one, you're not).
• How they're placed in the list once you fix the above.
• Don't cast malloc in C programs (read here for why).
• Fail to check the success of your scanf invoke.
• Fail to check the success of your malloc invoke

Before you get discouraged, things you did correctly:

• Did not mask a node pointer in a typedef
• Properly included a MCVE for review
• Prospected the things you may be doing wrong.

A very simple example of iterating three values into a linked list would look something like this:

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

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

typedef struct node NODE;

int main()
{
    NODE *head = NULL, *p;
    int i = 0;

    for(i = 0; i < 3; i++)
    {
        int inf;

        printf("Enter node value:");
        if (scanf("%d", &inf) == 1)
        {
            p = malloc(sizeof *p);
            if (p != NULL)
            {
                p->number = inf;
                p->next = head;
                head = p;
            }
            else
            {
                perror("Failed to allocate new node");
                return EXIT_FAILURE;
            }
        }
        else
        {
            // failed to read data. break
            break;
        }

        // report current linked list
        printf("%d", p->number);
        for (p=p->next; p; p = p->next)
            printf(" -> %d", p->number);
        fputc('\n', stdout);
    }

    // cleanup the linked list
    while (head)
    {
        p = head;
        head = head->next;
        free(p);
    }
    head = NULL;

    return 0;
}

Input

The values 1 2 3 are input upon being prompted:

Output

Enter node value:1
1
Enter node value:2
2 -> 1
Enter node value:3
3 -> 2 -> 1

Best of luck.

Answer 2

As said by Vamsi, you should use malloc to put the nodes on the heap. You also generally shouldn't cast the output of malloc, it isn't needed. And then you could play around with making a doubly-linked list, where you also have a prev pointer inside your struct.

Answer 3

You should use malloc() inside for loop. Since it is outside, same memory is being used.