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

Please format your code! It's hurting my eyes! – Fiddling Bits Feb 19 '15 at 18:56 — Fiddling Bits, Feb 19 '15 at 18:56
Looks like `node1` points to itself... – Fiddling Bits Feb 19 '15 at 18:58 — Fiddling Bits, Feb 19 '15 at 18:58
ok edited it. Please have another look. – Feb 19 '15 at 18:58 — , Feb 19 '15 at 18:58
Fails how? What do you mean by "eventually"? – Scott Hunter Feb 19 '15 at 18:59 — Scott Hunter, Feb 19 '15 at 18:59
I removed "eventually". Sorry for Grammar mistake as well. – Feb 19 '15 at 19:01 — , Feb 19 '15 at 19:01

score 1 · Answer 1 · edited May 23 '17 at 11:50

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.

score 0 · Accepted Answer · answered Feb 19 '15 at 19:01

0

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

answered Feb 19 '15 at 19:01

Vamsi

878
1
8
25

Thanks a ton buddy! Can't believe that was causing the error. :D – Feb 19 '15 at 19:02
Glad that I helped you. Please up vote and accept the answer if it worked for you. ;-) – Vamsi Feb 19 '15 at 19:09

score 0 · Answer 3 · answered Feb 19 '15 at 19:13

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.

Linked List insertion in Beginning

3 Answers3