Created attachment 112826 [details] [review]
glib_slist_foreach_remove.patch

Idea of the addition makes a lot of sense to me. (Really needs a g_list_ equivalent - the api's are supposed to be the same))

+GSList *
+g_slist_foreach_remove (GSList *list,
+		     GRemoveFunc func,
+		     gpointer user_data)
+{
+  GSList *ret, *prev, *cur;
+
+  g_return_val_if_fail (list != NULL, NULL);

Filtering a NULL list is valid.

+  g_return_val_if_fail (func != NULL, NULL);
+
+  ret = list;
+  prev = NULL;
+  cur = list;
+
+  while (cur != NULL)
+    {
+      if (func (cur->data, user_data))
+        {
+          GList *tmp;
+
+          /* if we're removing the head of the list, update
+             what we're returning to be the next item */
+          if (ret == cur)
+            {
+              g_assert (prev == NULL);
+              ret = cur->next;
+            }
+          /* otherwise, update the previous element to refer
+             to the next one */
+          else
+            {
+              g_assert (prev != NULL);

I don't see the need for this assert - it's going to segfault immediately anyways.

+              prev->next = cur->next;
+            }
+
+          tmp = cur;
+          cur = cur->next;

I think it's simpler if you have a next local in the loop. Something along the line of (untested):

===
prev = NULL;
for (l = list; l; ) {
     GSList *next = l->next;

     if (func(...)) {
         if (prev)
             prev->next = next;
         else
             list = next;
         g_slist_free_1(l);
     } else {
          prev = l;
     }

     l = next;
  }
===

+          tmp->next = NULL;
+          g_slist_free_1 (tmp);

Nulling out not needed.

+ * Specifies the type of functions passed to g_slist_foreach_remove

g_slist_foreach_remove()

+ * Returns: TRUE to remove the item, FALSE to keep it

%TRUE / %FALSE

+_filter_lt2 (gpointer item, gpointer user_data)

No reason to _prefix local static functions.

+    st = g_slist_nth (slist, i - 2);
+    g_assert (*((gint*) st->data) == i);

There is g_slist_nth_data, probably even better to have a helper get_nth_int() to make the tests more readable and verifiable.

-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/146.