CODEWITHSUNDEEP
linked-list
ZombieSpale
ZombieSpale

Reputation: 3

I'm getting into infinite while loop (linked lists c)

I'm stuck in a while loop (the one that prints "no" ):

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

typedef struct bill {
    int value;
    struct bill *next;
} list;

void printlist(list *head) { 
    list *temp = head;
    while (temp != NULL)
        printf("%d ",temp->value), temp = temp->next;
}

void insert_at_end(list *head, int value) {
    list *current, *first;
    first = malloc(sizeof(list));
        
    if (head == NULL)
        head = first;    
    else {
        current = head;
        while (current->next != NULL) {
            current = current->next;
            printf("no");
        }
    }
}

void main() {
    list *head = NULL;
    head = malloc(sizeof(list));
    for (int i = 0; i < 10; i++)
        insert_at_end(head, i);
    
    printlist(head);
}

I'm not sure why, but current never gets to NULL. I watched numerous videos and they all do the same:

while (current != NULL)
    current = current->next

...which should just get to NULL at one point, but it doesn't in my case.

Upvotes: 0

Views: 40

Answers (1)

trincot
trincot

Reputation: 350147

There are these issues:

  • When you do head = malloc(sizeof(list)), the members of that node are still undefined, including its next member. As a consequence, when you get into the else block in the insert_at_end function, the loop will access this undefined next pointer and bring about undefined behaviour.

  • Similar to the previous problem, also first does not get initialised with a value and a next member.

  • In the else block in the insert_at_end function there is no code that attaches the new node to the list.

  • In the if block in the insert_at_end function, the value of head is altered. But this just changes the value of a local variable -- the caller will not see this change. For that to happen you should alter the function parameter so that it is a pointer to the head pointer.

  • The creation of a head node in the main program seems unwaranted -- you should start with an empty list, i.e. with head equal to NULL.

  • void main is not correct. It should be int main, and the appropriate value should be returned by it.

Here is the correction to the two functions that have issues:

// This function accepts a pointer to a head-pointer, so the head-pointer
// can be changed and the caller will receive that change.
void insert_at_end(list **head, int value){
    list *current, *first;
    first = malloc(sizeof(list));
    first->value = value; // This was missing
    first->next = NULL; // This was missing
    if (*head == NULL) {
        *head = first;    
    } else {
        current = *head;
        while (current->next != NULL) {
            current = current->next;
        }
        current->next = first; // This was missing
    }
}

// The main method has an int return type
int main() {
    list *head = NULL;
    // Do not create a node here.
    for (int i = 0; i < 10; i++) {
        insert_at_end(&head, i); // pass pointer to head-pointer
    }
    printlist(head);
    return 0;
}

Upvotes: 1

Related Questions