Search code examples
cdebugginglinked-listmemory-address

Keep printing the last element in the linked list


I created a standard linked list in C. It asks the user to input a number, and program end if user input #. If the user inputs anything else the program will stop.

The problem is that my program runs forever and prints the normal list at first then keeping print the last element of the linked list. Hope someone could tell me where did I made mistake.

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

typedef struct node {
    int data;
    struct node *next;
} NodeT;

void freeLL(NodeT *list) {
    NodeT *p, *temp;
    p = list;
    while (p != NULL) {
        temp = p->next;
        free(p);
        p = temp;
    }
}

void showLL(NodeT *list) {
    NodeT *temp = list;
    temp = temp->next;
    printf("Done. The list is ");
    printf("%d", temp->data);
    temp = temp->next;
    //iterate the entire linked list and print the data
    while (temp != NULL) {
        printf("-->");
        printf("%d", temp->data);
        temp = temp->next; 
    }
}

NodeT *joinLL(NodeT *list, int v) {
    NodeT *current = list;
    NodeT *head;
    head->data = v;
    head->next = NULL;
    while (current->next != NULL) {
        current = current->next;
    }
    current->next = head;
    return head;
}

int main() {
    int data;
    NodeT *list = NULL;
    list = (NodeT *)malloc(sizeof(NodeT));
    printf("Enter a number: ");
    if (scanf("%d", &data) != 1) {
        printf("Done. ");
    } else {
        printf("Enter a number: ");
        joinLL(list, data);
        while (1 == scanf("%d", &data)) { 
            printf("Enter a number: ");
            joinLL(list, data);
        }
        showLL(list);
        freeLL(list);
    }
    
    return 0;
}

I believe the problem is in the joinLL function which add a new node at the end of the linked list.


Solution

  • The problem is you do not allocate elements in joinLL: only a single element in allocated in main().

    You should instead always allocate the element in joinLL and update the head pointer from the return value.

    Similary, freeLL should take a pointer to head and set it to NULL for consistency.

    Here is a modified version:

    #include <stdio.h>
    #include <stdlib.h>
    
    typedef struct node {
        int data;
        struct node *next;
    } NodeT;
    
    void freeLL(NodeT *p) {
        while (p != NULL) {
            NodeT *temp = p->next;
            free(p);
            p = temp;
        }
    }
    
    void showLL(const NodeT *list) {
        NodeT *p = list;
        printf("The list is ");
        if (p == NULL) {
            printf("empty");
        } else {
            printf(" %d", temp->data);
            while ((p = p->next) != NULL) {
                printf("--> %d", temp->data);
            }
        }
        printf("\n");
    }
    
    NodeT *joinLL(NodeT *head, int v) {
        NodeT *newp = malloc(sizeof(*p));
        NodeT *current;
    
        if (newp == NULL) {
            fprintf(stderr, "allocation failure\n");
            exit(1);
        }
        newp->data = v;
        newp->next = NULL;
        if (head == NULL) {
            return newp;
        }
        for (current = head; current->next != NULL; current = current->next)
            continue;
    
        current->next = newp;
        return head;
    }
    
    int main() {
        NodeT *list = NULL;
        for (;;) {
            int data;
            printf("Enter a number: ");
            if (scanf("%d", &data) != 1) {
                printf("Done. ");
                break;
            }
            list = joinLL(list, data);
        }
        showLL(list);
        freeLL(list);
        return 0;
    }