Linked List remove node function not working properly

Question

When I run the program below - the output I get is

 head node created
 node addded with value of 22 
 node addded with value of 22343 
 node addded with value of 7 
 22
 22343
 7
 last node has been removed 
 current last nodes value is 22343
 22
 22343
 7

This issue is that , if the last node in the list is stated to have a data value of 22343 when the removenode function is called , how is it possble that the last value printed is 7 and not 22343 - when the traverse function is called? despite the fact that the *next pointer of the last node is set to NULL in the removenode funtion.

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

struct node_n
{
    int value;
    struct node *next;
};
typedef struct node_n node;

node* makeheadnode()
{
    node *temp = (node*)malloc(sizeof(node));
    temp->next = NULL;
    temp->value = NULL;
    printf("\n head node created");
    return temp;

}

void addnode(node *target, int data)
{
    while (target->next != NULL)
    {
        target = target->next;
    }
    target->next = (node*)malloc(sizeof(node));
    target = target->next;
    target->value = data; 
    target->next = NULL;   
    printf("\n node addded with value of %d ",target->value);
}

int removenode(node *target)
{
    node *temp;

    if(target->next == NULL)
    {
        printf("\n only one node is present");
        return 1;
    }

    while(target -> next != NULL)
    {
        temp = target -> next;
        if(temp -> next == NULL)
        {
            target->next == NULL;
            printf("\n last node has been removed \n current last nodes value is %d",target->value);
            return 1;
        }
        else 
        {
            target = target-> next;
        }
    }
}

int traverse(node *target)
{
    if(target->next == NULL)
    {
        printf("\n this list is empty");
        return 1;
    }
    while (target->next != NULL)
    {
        target = target -> next;
        printf("\n %d", target -> value);
    }
    return 1;
    
}

int main()
{

    node *head = makeheadnode();
    
    addnode(head,22);

    addnode(head,22343);

    addnode(head,007);

    traverse(head);

    removenode(head);

    traverse(head);

}

Aside: `temp->value = 0;` makes more sense than `temp->value = NULL;`. — chux - Reinstate Monica, Jul 01 '22 at 20:53
`NULL` is probably 0 as an `int` on your platform anyway. You need to pick a value not to be able to store (e.g. -1 if you only care about the positive range) or store information about presence/absence separately (e.g. a boolean field). — Ry-, Jul 01 '22 at 20:57
Fyi, this member is wrong: `struct node *next;` . There is no `struct node`, there is `struct node_n`. Given that, "not-working" can only mean "not-compiling". — WhozCraig, Jul 01 '22 at 20:59

score 2 · Accepted Answer · answered Jul 01 '22 at 20:59

The problem is a typo. In your removenode function, you have:

target->next == NULL;

Where it should be:

target->next = NULL;

If you compile with warnings on, i.e. -Wall or even -Werror, the compiler should have warned you with a message like equality comparison result unused.

Furthermore, since you're removing the node, you should free it so you don't have any memory leaks. Setting a pointer to NULL does not free the pointer. Many, including myself, also see it as good practice to set the pointer to NULL after freeing.

free(target->next)
target->next = NULL

For more info on that, see this post

Linked List remove node function not working properly

1 Answers1