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.

CodePudding user response：

Your program keeps running because of a memory access error, you did not allocate memory for your head(you set a pointer, but use it directly without initializing it)

Change to this may solve the problem:

head=(NodeT*)malloc(sizeof(NodeT));
if(NULL==head)
{
   // failed : do something...
   return NULL;
}
head->data=v;
head->next=NULL;

When I just tested it, I found that there was another problem:

    list = (NodeT*)malloc(sizeof(NodeT));

malloc will not be initialize your list, so the value that your list->next initially points to is uncertain.

in c, malloc does not need to be cast.

CodePudding user response：

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