C single-linked list with remove Can you please point out any problems with this C forward list with remove node functionality?

I believe this can be way simpler ( removeNode function ) but I somehow cannot grasp it

#include &lt;stdlib.h&gt;
#include &lt;stdio.h&gt;
#include &lt;assert.h&gt;

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

node* newNode(int val)
{
  node* n = (node*)malloc(sizeof(node));
  n-&gt;val = val;
  n-&gt;next = NULL;
  return n;
}

void printList(node* head)
{
  if (head == NULL) return;

  do
  {
    printf("%d, ", head-&gt;val);
    head = head-&gt;next;
  } while (head-&gt;next);

  printf("\n");
}

void clean(node* head)
{
  while (head)
  {
    node* curr = head;
    if (head-&gt;next == NULL)
    {
      free(head);
      return;
    }

    head = head-&gt;next;
    free(curr);
  }
}

void insert(node* head, int val)
{
  while (head)
  {
    if (head-&gt;next)
    {
      head = head-&gt;next;
    }
    else
    {
      head-&gt;next = newNode(val);
      return;
    }
  }
}

void removeIf(node** head, int val)
{
  if (head == NULL) return;
  if (*head == NULL)
  {
    head = NULL;
    return;
  }

  while (*head &amp;&amp; (*head)-&gt;val == val)
  {
    if ((*head)-&gt;next == NULL)
    {
      free(*head);
      *head = NULL;
      return;
    }
    node* tofree = *head;
    *head = (*head)-&gt;next;
    free(tofree);
  }

  node* curr = (*head)-&gt;next;
  node* prev = *head;

  while (curr)
  {
    if (curr-&gt;next == NULL) return;

    if (curr-&gt;val == val)
    {
      prev-&gt;next = curr-&gt;next;
      node* tofree = curr;
      curr = curr-&gt;next;
      free(tofree);
    }
    else
    {
      prev = prev-&gt;next;
      curr = curr-&gt;next;
    }
  }
}

int size(node* root)
{
  if (root == NULL) return 0;
  int sz = 1;
  while (root-&gt;next)
  {
    root = root-&gt;next;
    ++sz;
  }

  return sz;
}

void tests();

int main()
{
  tests();
}

/*
 * Here are the tests for lists functionalities
 */
void tests()
{
  {
    node* root = newNode(1);
    insert(root, 2);
    assert(size(root) == 2);
    clean(root);
  }
  {
    node* root = newNode(8);
    insert(root, 2);
    insert(root, 2);
    assert(size(root) == 3);
    clean(root);
  }
  {
    node* root = newNode(8);
    insert(root, 123);
    insert(root, 123);
    insert(root, 123);
    insert(root, 123);
    assert(size(root) == 5);
    clean(root);
  }
  {
    node* root = newNode(8);
    insert(root, 123);
    assert(size(root) == 2);
    removeIf(&amp;root, 1);
    assert(size(root) == 2);
    clean(root);
  }
  {
    node* root = newNode(1);
    insert(root, 1);
    insert(root, 1);
    insert(root, 1);
    insert(root, 2);
    insert(root, 3);
    insert(root, 3);
    insert(root, 4);
    insert(root, 4);
    insert(root, 5);
    assert(size(root) == 10);
    removeIf(&amp;root, 1);
    assert(size(root) == 6);
    clean(root);
  }
  {
    node* root = newNode(1);
    insert(root, 1);
    insert(root, 1);
    insert(root, 1);
    insert(root, 2);
    insert(root, 3);
    insert(root, 3);
    insert(root, 4);
    insert(root, 4);
    insert(root, 5);
    assert(size(root) == 10);
    removeIf(&amp;root, 3);
    assert(size(root) == 8);
    clean(root);
  }
  {
    node* root = newNode(1);
    insert(root, 1);
    assert(size(root) == 2);
    removeIf(&amp;root, 1);
    assert(size(root) == 0);
    clean(root);
  }
  {
    node* root = newNode(1);
    insert(root, 1);
    insert(root, 1);
    insert(root, 1);
    assert(size(root) == 4);
    removeIf(&amp;root, 1);
    assert(size(root) == 0);
    clean(root);
  }
}
No need to peek ahead for the loop condition. Especially not AFTER incrementing it already. Coincidence that this function was not part of the test suite?

void printList(node* head)
{
  if (head == NULL) return;

  while(head)
  {
    printf("%d, ", head-&gt;val);
    head = head-&gt;next;
  };

  printf("\n");
}




Too much redundant code:

void clean(node* head)
{
  while (head)
  {
    node* curr = head;
    head = head-&gt;next;
    free(curr);
  }
}




In void insert(node* head, int val), no need to branch inside the loop. Do it step by step. First find the tail, then append.



void removeIf(node** head, int val) would become already much simpler if it did just add a temporary head to the list, as the special handling for removing a chain at the head wouldn't be necessary in that case. Finally destroy the temporary head again, and overwrite head unconditionally.



In int size(node* root), why call it root all of a sudden and no longer head?

Also, unnecessary lookahead and branching again:

int size(node* head)
{
  int sz = 0;
  while (head)
  {
    ++sz;
    head = head-&gt;next;
  }
  return sz;
}




In terms of code style only one remark, and that is on the location of the * when working with pointer types.

I know, placing it next to the type feels more natural, but it's semantically inaccurate, and it will trip you the first time you attempt to declare multiple variables at once. Placing it where there compiler would sort it helps preventing these mistakes.