Deleting a specific node from singly linked list in C

Question

I have made a singly linked list in C and I am trying to code a function which deletes a specific element from the list based on the integer value provided which corresponds to a particular node.

void deleteNodeVar(int val)
{
    struct node *t, *temp, *temp2;

    t = START;
    while(t -> info != val)
    {
        if(t -> ptr == NULL) { break; }
        t = t -> ptr;
    }
    printf("\nfound the val to be %d\n",t -> info);
    temp = t -> ptr;
    t -> ptr = temp -> ptr;
    printf("now will free the node with value %d \n",t -> info);
    free(t);

}

After some debugging I found: The function is working properly as it detects and deletes the node successfully

but printing the entire list gives weird results.

full code:

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

struct node
{
    int info;
    struct node *ptr;
};

struct node *START = NULL;

struct node* createNode()
{
    struct node *p;
    p = (struct node*) malloc(sizeof(struct node));
    return p;
}

//inserting node
void insertNode(int val)
{
    struct node *temp, *t;
    temp = createNode();
    temp -> info = val;
    temp -> ptr = NULL;

    if(START == NULL){ START = temp; }
    else
    {
        t = START;
        while(t -> ptr != NULL)
        {
            t = t -> ptr;
        }
        t -> ptr = temp;
    }   
}


void pop()
{
    struct node *t;
    t = START;
    if(START == NULL) { printf("THE LIST IS EMPTY\n"); }
    else
    {
        START = START -> ptr;
        free(t);
    }
}

//here are the 2 functions below in which I have problem
void deleteNodeVar(int val)
{
    struct node *t, *temp, *temp2;

    t = START;
    while(t -> info != val)
    {
        if(t -> ptr == NULL) { break; }
        t = t -> ptr;
    }
    printf("\nfound the val to be %d\n",t -> info);
    temp = t -> ptr;
    t -> ptr = temp -> ptr;
    printf("now will free the val with value %d \n",t -> info);
    free(t);

}

void viewList()
{
    struct node *t, *temp;
    t = START;

    while(t != NULL)
    {
        printf("%d -> ",t->info);
        t = t -> ptr;
    }

}

int main() 
{

    insertNode(10);
    insertNode(20);
    insertNode(40);
    insertNode(100);

    viewList();
    int v;
    printf("\nEnter to delete: ");
    scanf("%d",&v);
    deleteNodeVar(v);
    viewList();

}

Here is the screenshot of the output I am getting after trying to print the list after deleting node with value 40:

It is running into an infinite loop with these repeating values !

See community addition in this answer: https://stackoverflow.com/a/22122095/4581301 — user4581301, May 01 '20 at 16:33
Is there a specific reason why you tagged this as C++? from the question as stated, this seems to be only a C thing. — , May 01 '20 at 16:33
@Frank it was more of a data structure thing and C and C++ only have some differences so I included C++. — Aquib Shaikh, May 02 '20 at 08:23

score 1 · Answer 1 · answered May 01 '20 at 17:03

For starters it is a bad idea to use a global variable as a pointer to the heda node of the list and when functions depend on a global variable. In this case you are unable to create more than one list.

Nevertheless the function deleteNodeVar can be defined the following way

int deleteNodeVar(int val)
{
    node *prev = NULL, *current = START;

    while ( current != NULL && current->info != val )
    {
        prev = current;
        current = current->ptr;
    }

    int success = current != NULL;

    if ( success )
    {
        if ( prev == NULL )
        {
            START = START->ptr;
        }
        else
        {
            prev->ptr = current->ptr;
        }

        free( current );
    }

    return success;
}

score 0 · Answer 2 · answered May 01 '20 at 17:31

According to your delete function if the node is not present in the list which you want to delete but even after your delete function will give a message that

found the val to be

which is totally wrong .... so you can write your delete function like this

void deleteNodeVar(int val)
{
    node *p ='\0', *n = START;
     int flag=0;

    while ( n != '\0' && n->info != val )
    {
        p =n;
        n =n->ptr;
    }
     if(n!='\0')
          flag=1;


    if ( flag )
    {
        if ( p == '\0' )
        {
            START = START->ptr;
        }
        else
        {
            p->ptr = n->ptr;
        }
              printf("\nfound the val to be %d\n",n-> info);
        free( n );
    }

    else
        printf("The node is not present list\n");
}

Prefer a `NULL` to `'\0'` when nulling a pointer. The intent of the code is more obvious. `if(n!='\0') flag=1;if ( flag )` is mostly redundant. `flag` is only used to trigger the second `if`, so, `if(n!='\0')` does everything that needs to be done. — user4581301, May 01 '20 at 18:25

Deleting a specific node from singly linked list in C

2 Answers2