Linked list unable to print all elements

Viewed 52

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();
    }
}
2 Answers

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

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