Error deleting name from linked list in c

Question

So, the program takes the names entered by the user, displays those names and removes a name that the user wants. The problem is that when I type a name that wasn't entered, it removes the last name that was entered in the list.

Struct:

struct node
{
    char name[50];
    struct node *next;
}*node;

remove function:

void remove(){
   char nameToDelete[50];
   struct node *temp, *previous;
   temp = node;

   printf("What is the name you wish to delete?\n");
   scanf("%s", nameToDelete);

   for ( ; temp->next != NULL; temp = temp->next )
   {
      previous = temp;
      if(strcmp(nameToDelete, temp->name)==0)
      {
         break;
      }
   }

   if ( temp == node )
   {
      node = temp->next;
   }
   else
   {
      previous->next = temp->next;
   }

   free(temp);
   printf("%s was deleted successfully\n", nameToDelete);
}

.

When you exit the loop, `previous` and `temp` are the same, right? Can you see why that won't work? You need to set `previous` *before* advancing `temp`. — Tom Karzes, Sep 13 '21 at 20:27
You have to check whether you reached the end of the list without finding the name. — Barmar, Sep 13 '21 at 20:31
`scanf("%s", nameToDelete);` is [as dangerous as `gets`](https://stackoverflow.com/questions/1694036/why-is-the-gets-function-so-dangerous-that-it-should-not-be-used). Limit your input length to avoid exceeding your buffer size: `scanf("%49s", nameToDelete);` — Oka, Sep 13 '21 at 20:42

Vlad from Moscow · Accepted Answer · 2021-09-13T20:55:19.080

In this loop

for ( ; temp->next != NULL; temp = temp->next )
{
   previous = temp;
   if(strcmp(nameToDelete, temp->name)==0)
   {
      break;
   }
}

if the name to be deleted is not found then the third expression of the loop temp = temp->next is evaluated and after the loop the pointer temp points to the last node that then is deleted in this code snippet

//...
else
{
   previous->next = temp->next;
}

free(temp);

Another problem is if the name to be deleted is found then the pointers previous and temp are equal each other due to the break statement

Also the function should initially check whether the pointer node is equal to NULL.

The function can be declared and defined at least the following way

int remove( void )
{
    char nameToDelete[50];

    printf("What is the name you wish to delete?\n");
    scanf("%s", nameToDelete);

    struct node *temp = node, *previous = NULL;

    while ( temp != NULL && strcmp( nameToDelete, temp->name ) != 0 )
    {
        previous = temp;
        temp = temp->next;
    }

    int success = temp != NULL;

    if ( success )
    {
        if ( previous == NULL ) node = node->next;
        else previous->next = temp->next;
    
        free( temp );
    }

    return success;
}

though it is better to exclude the code where the user is asked to enter a name. It is the caller of the function should provide the name to be deleted as a function argument. In this case the function will look like

int remove( const char *nameToDelete )
{
    struct node *temp = node, *previous = NULL;

    while ( temp != NULL && strcmp( nameToDelete, temp->name ) != 0 )
    {
        previous = temp;
        temp = temp->next;
    }

    int success = temp != NULL;

    if ( success )
    {
        if ( previous == NULL ) node = node->next;
        else previous->next = temp->next;
    
        free( temp );
    }

    return success;
}

Error deleting name from linked list in c

1 Answers1