Created attachment 204549 [details] [review]
array: Add a clear function

Like GPtrArray has a "free function" that can be used to free memory
associated to each pointer in the array, GArray would benefit from
having a "clear function" that can be used to clear the content of
each element of the array when it's removed, or when the entire array
is freed.

Review of attachment 204549 [details] [review]:

::: glib/garray.c
@@ +211,3 @@
+ * @clear_func: a function to clear each item in the array
+ *
+ * Sets a function to clear each item in the @array.

How about:

"Sets the function to clear an element of @array."

@@ +349,3 @@
+          gint i;
+
+          for (i = 0; i < array->len; i++)

Note that array->len is unsigned.  You should stop and think any time when comparing signed to unsigned.  In this case i is never negative, so it's OK, but it's probably better to use 'guint i';

@@ +656,3 @@
+
+      for (i = 0; i < length; i++)
+        array->clear_func (g_array_elt_pos (array, index_ + i));

And here we're adding a signed and unsigned.  Better to make i explicitly unsigned too.

::: glib/tests/array-test.c
@@ +330,3 @@
+
+  g_array_remove_index (garray, 9);
+  g_assert_cmpint (num_clear_func_invocations, ==, 1);

Hmm...a test which actually used say g_malloc() for data is more likely to fail if someone broke GArray - the free func could be called for the wrong address and this test wouldn't catch it.

Comment on attachment 204549 [details] [review]
array: Add a clear function

>+void
>+g_array_set_clear_func (GArray         *array,
>+                        GDestroyNotify  clear_func)

This isn't a "real" GDestroyNotify though, since it only frees the contents, not the pointer itself... I guess it probably doesn't matter, but the docs should probably be clearer. (That it receives a pointer to the array item rather than the array items itself, and that it shouldn't free the pointer.)


Also, g_array_set_size() needs to free elements when shrinking the array. (Just make it use g_array_remove_range() like g_ptr_array_set_size() does.)

The following fix has been pushed:
c602a5f array: Add a clear function

Created attachment 206050 [details] [review]
array: Add a clear function

Like GPtrArray has a "free function" that can be used to free memory
associated to each pointer in the array, GArray would benefit from
having a "clear function" that can be used to clear the content of
each element of the array when it's removed, or when the entire array
is freed.