# Removing nodes from a linked list

This is my second iteration of a function to remove all nodes containing value 'x'. The previous iteration is here.

This function takes as input the head of a list and has to delete all the nodes that contain a given value taken. Is this an improvement on the previous iteration? Is there anything else that can be improved?

typedef struct node {
int val;
struct node *next;
} node_t;


node_t *rimuovi(node_t *head, int x){

printf("Lista vuota, finito!\n");
}

printf("La lista adesso e' vuota. Finito\n");
}
else {
}
}

curr = temp->next;

while (curr != NULL){
if (curr->val == x){
if(curr->next != NULL){
temp->next = curr->next;
free (curr);
curr = temp->next;
}
else {
temp->next = NULL;
}
}
else {
temp = curr;
curr = curr->next;
}
}

}

• This looks like the code version before you modified it. Feb 11, 2020 at 15:20
• yes cause they said me to post a new post not to modify the original one Feb 11, 2020 at 16:06
• The suggestion was that you ask a new question with the new version of your code. That way you can get feedback on the new version, without making the previous question confusing. I've edited your question to reflect a request for a followup review. If it doesn't match what you're looking for, please revert the change. Feb 11, 2020 at 20:03

Bug: Returning a free'd pointer!

After freeing a pointer, do not use it.

        free(head);
printf("La lista adesso e' vuota. Finito\n");
return NULL;


Is there anything else that can be improved?

Rather than 2 or 3 cases to handle an empty list, list of only matches or mixed list, consider a code simplification.

Create a temporary pre-head node and assign its next to the head. Only 1 case needed. This approach is reasonable when node_t is not large.

Some untested code.

node_t *rimuovi(node_t *head, int x) {

while (p->next) {
if (p->next->val == x) {
node_t *next = p->next->next;
free(p->next);
p->next = next;
} else {
p = p->next;
}
}
}


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


There's a lot of duplication between these branches:

while(head->val == x){
printf("La lista adesso e' vuota. Finito\n");
}
else {
}
}


We always free(head), and we probably ought to set head = NULL in the first branch, rather than leaving it pointing to freed memory.
Therefore:

while (head->val == x) {