Why does a linked list with a double pointer cause an error?

Question

I'm asking you a question because the assignment I was doing didn't work out. The structure is a common link list, declaring the head pointer in the main and passing the address value of the head pointer as a parameter to the function. The global variable top is used to determine where the current data is located.

The code currently below will detect only errors when executed.

Structure:

struct ListNode{
    int data;
    struct ListNode* link;
};
int top = 0;

code:

void DisplayList(ListNode** head){
    if(*head == NULL){
        printf("List = Empty\n");
    }
    else{
        printf("List = ");
        for(;(*head) != NULL; *head = (*head)->link){
            printf("%d ",(*head)->data);
        }
    }
    printf("\n");
}

void AddList(ListNode** head){
    ListNode* temp = (ListNode*)malloc(sizeof(ListNode));
    int num;
    printf("Data register) ");
    scanf("%d",&num);
    temp->data = num;
    temp->link = NULL;
    top++;

    if(*head == NULL){  
        *head = temp;
    }
    else{
        for(;(*head)->link != NULL; *head = (*head)->link){}    
        (*head)->link = temp;
    }
    DisplayList(head);
}

the expected result:

Data register) 10 List = 10

Data register) 20 List = 10 20

Data register) 30 List = 10 20 30

What do you mean by the assignment we were working on is not working out? We're not working on any assignment. — Barmar, Oct 26 '19 at 10:21

Barmar · Answer 1 · 2019-10-26T10:33:51.140

You shouldn't modify *head in the loops. You need to use a local variable to step through the list, otherwise you're changing the caller's variable to point to the end of the list.

void DisplayList(ListNode** head){
    if(*head == NULL){
        printf("List = Empty\n");
    }
    else{
        printf("List = ");
        for(ListNode *step = *head;step != NULL; step = step->link){
            printf("%d ",step->data);
        }
    }
    printf("\n");
}

void AddList(ListNode** head){
    ListNode* temp = (ListNode*)malloc(sizeof(ListNode));
    int num;
    printf("Data register) ");
    scanf("%d",&num);
    temp->data = num;
    temp->link = NULL;
    top++;

    if(*head == NULL){  
        *head = temp;
    }
    else{
        ListNode *step = *head;
        for(;step->link != NULL; step = step->link){}    
        step->link = temp;
    }
    DisplayList(head);
}

M Oehm · Answer 2 · 2019-10-26T10:49:21.660

Your error is in how you traverse the list:

for(; (*head)->link != NULL; *head = (*head)->link) {}

At the beginning, *head is the head of the list from the calling function. By assigning to it, you overwrite it continuously, until it becomes null.

Instead, you should assign to head: It holds the address of the head node in the calling function at first and the address of the link pointer of the previous node in subsequent iterations:

for(; (*head)->link != NULL; head = &(*head)->link) {}

After this loop, head holds the address of the pointer where the new node should be stored. Assign the new node to that pointer directly:

*head = temp;

This will update the head pointer of the calling function when the list was empty and the link member of the previous node otherwise. You don't have to treat the case where the list is empty specially.

The insertion function might now look like this:

void AddList(ListNode** head, int num)
{
    ListNode* temp = malloc(sizeof(ListNode));

    temp->data = num;
    temp->link = NULL;

    while (*head) head = &(*head)->link);
    *head = temp;
}

(In my opinion, reading the input and printing the list shoudl not be part of te insertion function.)

Regarding your printing function: Seeing a function like that:

void DisplayList(ListNode** head)

raises a red flag: Passing a pointer to node pointer signals that you want to modify the list (and you inadvertently do that), but printing the list only inspects it. Change the signature to

void DisplayList(const ListNode* head)

then use head instead of (*head) in the function. (Rule of thumb: If you never use *head = ... somewhere in the list, you don't need a pointer to node pointer.)

I find your answer confusing, using `head` as a local variable. It also doesn't handle the empty list as the new element must be put in `*head`. — Paul Ogilvie, Oct 26 '19 at 10:37
@PaulOgilvie: I does handle insertion to an empty list, but the assignment must be `*head = temp`, which my answer doesn't explian properly. — M Oehm, Oct 26 '19 at 10:39

Why does a linked list with a double pointer cause an error?

the expected result:

2 Answers2