Linked list unable to print all elements

Question

I'm trying to print the linked list to which I prompt for user input. This code below is not printing the whole list, only the last element at a time. I don't seem to find the bug. Can you please take a look at it?

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

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

struct Node *head;

void Insert(int x) {
    struct Node *temp = (struct Node *)malloc(sizeof(struct Node));
    temp->data = x;
    temp->next = NULL;
    head = temp;
};

void Print() {
    struct Node *temp = head;
    printf("Linked list is: ");
    while (temp != NULL) {
        printf("%d ", temp->data);
        temp = temp->next;
    }
    printf("\n");
};

int main() {
    head = NULL;
    int i, x;
    for (i = 1; i <= 10; i++) {
        if (i == 1) {
            printf("Enter 1st number: \n");
        } else if (i == 2) {
            printf("Enter 2nd number: \n");
        } else {
            printf("Enter %dth number: \n", i);
        }
        scanf("%d", &x);
        Insert(x);
        Print();
    }
}

Does this answer your question? [Inserting Node Linked List C](https://stackoverflow.com/questions/53452734/inserting-node-linked-list-c) — Hussien Mostafa, May 30 '22 at 08:03
I have heard that (but not sure why) it is not a good practice to cast the resulting address of malloc, calloc or realloc functions in C PROGRAMMING (line 12th of your program, where you're mallocing the pointer temp), and sometimes it would result a buggy executable. But I'm not saying that it's the culprit of the problem you're experiencing... — , May 30 '22 at 08:36

Petr Skocik · Answer 1 · 2022-05-30T11:42:02.700

temp->next = NULL; is the culprit. It should be temp->next = head;.

Another (more cornercase) issue is that your code fails to check for errors in malloc and scanf.

Edit in response to comment:

If you want to append (as opposed to prepend), you'll need to keep a tail pointer for forward traversal and then either use a dummy first node (avoids a branch) or special-case an insert to an empty list.

Example of both (with simplistic error handling via exit(1)) in one piece of code:

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

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

#define DUMMYFIRST 1 //change to 0 to compile the other variant

#if DUMMYFIRST
    struct Node dummyfirst;
    struct Node *head=&dummyfirst;
#else
    struct Node *tail,*head=0;
#endif

void Insert(int x) {
    struct Node *newnode = malloc(sizeof(struct Node));
    //don't cast the result of malloc in C
    //https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc

    if(!newnode) { perror("malloc"); exit(1); }
    newnode->data = x;
    newnode->next = 0;

    #if !DUMMYFIRST
        if(!tail) tail = head = newnode;
        else head->next = newnode;
    #else
        head->next = newnode;
    #endif

    head = newnode;
};

void Print() {
    #if DUMMYFIRST
        struct Node *newnode = dummyfirst.next;
    #else
        struct Node *newnode = tail;
    #endif
    printf("Linked list is: ");
    while (newnode != NULL) {
        printf("%d ", newnode->data);
        newnode = newnode->next;
    }
    printf("\n");
};

int main() {
    int i, x;
    for (i = 1; i <= 10; i++) {
        if (i == 1) {
            printf("Enter 1st number: \n");
        } else if (i == 2) {
            printf("Enter 2nd number: \n");
        } else {
            printf("Enter %dth number: \n", i);
        }
        if(1!=scanf("%d", &x)) exit(1);
        Insert(x);
        Print();
    }
}

A more library friendly approach to handling errors would be to propagate the error to the caller, i.e., instead of exiting with an error message right away, you'd change the return value from void to something indicating the error, e.g. so that the caller could check and decide what to do (print it, print it in a localized version, try a different algorithm...)

E.g.:

struct Node *Insert(int x) {
    struct Node *newnode = malloc(sizeof(struct Node));
    //don't cast the result of malloc in c
    //https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc

    if(!newnode) return NULL;
    //...
};
//...
//calling code:
    if(!Insert(x)) perror("Insert"),exit(1);

`malloc` returns `NULL` on an error and the error code (`ENOMEM` in most (all?) cases) is stored in `errno` (printable with e.g., `perror`). With `scanf`, it's more complicated. On error, it sets `errno` and returns `EOF`, but `EOF` also indicates end of file (then no errno is set) and you might need to differentiate between the two with `ferror(stdin)` or `feof(stdin)`. It's all in the manpages. — Petr Skocik, May 30 '22 at 08:06
@Ori If you get errors in those functions and don't handle them, then your code is technically undefined and can do anything. (Practically, a malloc failure will lead to a segfault on NULL and a scanf failure to you storing junk values in the nodes. Most likely.) — Petr Skocik, May 30 '22 at 08:11
@Ori It's common to propagate the error upwards (have Insert return an error indicator on malloc failure and have the Insert caller check that) and if they're detected, print them in the top-level caller (malloc). That way you can build a library out of those helper functions without that library being unnecessary noisy in the standard error stream. — Petr Skocik, May 30 '22 at 08:19
Can you explain them with examples? I don't get it because I'm not a pro. — , May 30 '22 at 08:22
@Ori Don't bother too much about error checking for `malloc` in toy programs. It's very unlikely to happen, at least on a desktop platform. Also checking for errors with `scanf` is also more or less pointless for toy programs, and in real world programs you wouldn't use `scanf` anyway for user input. — Jabberwocky, May 30 '22 at 08:35
@Ori Added an example of appending at the end and of error propagation through return values. — Petr Skocik, May 30 '22 at 11:38
Ah, you're being very kind. But, I edited/added a few places, and figured it out by myself. — , May 30 '22 at 11:41

chqrlie · Accepted Answer · 2022-05-30T08:37:07.117

When you insert the new node, you do not link the rest of the list, instead of temp->next = NULL; you should write

    temp->next = head;

To ensure defined behavior, you should check for memory allocation failure and invalid input.

Also remove the dummy ; after the function bodies.

Here is a modified version:

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

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

struct Node *head;

int Insert(int x) {
    struct Node *temp = malloc(sizeof(*temp));
    if (temp) {
        temp->data = x;
        temp->next = head;
        head = temp;
        return 1;
    } else {
        return 0;
    }
}

void Print(void) {
    struct Node *temp = head;
    printf("Linked list is: ");
    while (temp != NULL) {
        printf("%d ", temp->data);
        temp = temp->next;
    }
    printf("\n");
}

int main() {
    static char suffix[4][3] = { "th", "st", "nd", "rd" };
    int i, x;
    for (i = 1; i <= 10; i++) {
        int suff = (i >= 1 && i <= 3) ? i : 0;
        printf("Enter %d%s number:\n", i, suffix[suff]);
        if (scanf("%d", &x) != 1) {
            fprintf(stderr, "invalid or missing input\n");
            break;
        }
        if (!Insert(x)) {
            fprintf(stderr, "cannot allocate memory for Node\n");
            return 1;
        }
        Print();
    }
    return 0;
}

@Ori: sorry, my bad, I still had the `temp->next = NULL;` in the code. Answer updated. — chqrlie, May 30 '22 at 08:23
I also updated the code so `Insert` returns a success indicator and `main` decides what to do in case of failure. — chqrlie, May 30 '22 at 08:37

Linked list unable to print all elements

2 Answers2