Merge branch '1374-array-tests' into 'master'

Resolve "Fix g_array_insert_vals() with an index off the end of the array"

Closes #1374

See merge request GNOME/glib!12

glib/garray.c

@@ -439,11 +439,14 @@ g_array_append_vals (GArray       *farray,
 /**
  * g_array_prepend_vals:
  * @array: a #GArray
- * @data: (not nullable): a pointer to the elements to prepend to the start of the array
+ * @data: (nullable): a pointer to the elements to prepend to the start of the array
- * @len: the number of elements to prepend
+ * @len: the number of elements to prepend, which may be zero
  *
  * Adds @len elements onto the start of the array.
  *
+ * @data may be %NULL if (and only if) @len is zero. If @len is zero, this
+ * function is a no-op.
+ *
  * This operation is slower than g_array_append_vals() since the
  * existing elements in the array have to be moved to make space for
  * the new elements.
@@ -498,11 +501,19 @@ g_array_prepend_vals (GArray        *farray,
  * g_array_insert_vals:
  * @array: a #GArray
  * @index_: the index to place the elements at
- * @data: (not nullable): a pointer to the elements to insert
+ * @data: (nullable): a pointer to the elements to insert
  * @len: the number of elements to insert
  *
  * Inserts @len elements into a #GArray at the given index.
  *
+ * If @index_ is greater than the array’s current length, the array is expanded.
+ * The elements between the old end of the array and the newly inserted elements
+ * will be initialised to zero if the array was configured to clear elements;
+ * otherwise their values will be undefined.
+ *
+ * @data may be %NULL if (and only if) @len is zero. If @len is zero, this
+ * function is a no-op.
+ *
  * Returns: the #GArray
  */
 /**
@@ -532,6 +543,11 @@ g_array_insert_vals (GArray        *farray,
   if (len == 0)
     return farray;
 
+  /* Is the index off the end of the array, and hence do we need to over-allocate
+   * and clear some elements? */
+  if (index_ >= array->len)
+      return g_array_append_vals (g_array_set_size (farray, index_), data, len);
+
   g_array_maybe_expand (array, len);
 
   memmove (g_array_elt_pos (array, len + index_),

glib/tests/array-test.c

@@ -30,6 +30,42 @@
 #include <string.h>
 #include "glib.h"
 
+/* Test data to be passed to any function which calls g_array_new(), providing
+ * the parameters for that call. Most #GArray tests should be repeated for all
+ * possible values of #ArrayTestData. */
+typedef struct
+{
+  gboolean zero_terminated;
+  gboolean clear_;
+} ArrayTestData;
+
+/* Assert that @garray contains @n_expected_elements as given in @expected_data.
+ * @garray must contain #gint elements. */
+static void
+assert_int_array_equal (GArray     *garray,
+                        const gint *expected_data,
+                        gsize       n_expected_elements)
+{
+  gsize i;
+
+  g_assert_cmpuint (garray->len, ==, n_expected_elements);
+  for (i = 0; i < garray->len; i++)
+    g_assert_cmpint (g_array_index (garray, gint, i), ==, expected_data[i]);
+}
+
+/* Iff config->zero_terminated is %TRUE, assert that the final element of
+ * @garray is zero. @garray must contain #gint elements. */
+static void
+assert_int_array_zero_terminated (const ArrayTestData *config,
+                                  GArray              *garray)
+{
+  if (config->zero_terminated)
+    {
+      gint *data = (gint *) garray->data;
+      g_assert_cmpint (data[garray->len], ==, 0);
+    }
+}
+
 static void
 sum_up (gpointer data,
 	gpointer user_data)
@@ -42,38 +78,46 @@ sum_up (gpointer data,
 /* Check that expanding an array with g_array_set_size() clears the new elements
  * if @clear_ was specified during construction. */
 static void
-array_new_cleared (void)
+array_set_size (gconstpointer test_data)
 {
+  const ArrayTestData *config = test_data;
   GArray *garray;
   gsize i;
 
-  garray = g_array_new (FALSE, TRUE, sizeof (gint));
+  garray = g_array_new (config->zero_terminated, config->clear_, sizeof (gint));
   g_assert_cmpuint (garray->len, ==, 0);
+  assert_int_array_zero_terminated (config, garray);
 
   g_array_set_size (garray, 5);
   g_assert_cmpuint (garray->len, ==, 5);
+  assert_int_array_zero_terminated (config, garray);
 
-  for (i = 0; i < 5; i++)
+  if (config->clear_)
-    g_assert_cmpint (g_array_index (garray, gint, i), ==, 0);
+    for (i = 0; i < 5; i++)
+      g_assert_cmpint (g_array_index (garray, gint, i), ==, 0);
 
   g_array_unref (garray);
 }
 
-/* As with array_new_cleared(), but with a sized array. */
+/* As with array_set_size(), but with a sized array. */
 static void
-array_new_sized_cleared (void)
+array_set_size_sized (gconstpointer test_data)
 {
+  const ArrayTestData *config = test_data;
   GArray *garray;
   gsize i;
 
-  garray = g_array_sized_new (FALSE, TRUE, sizeof (gint), 10);
+  garray = g_array_sized_new (config->zero_terminated, config->clear_, sizeof (gint), 10);
   g_assert_cmpuint (garray->len, ==, 0);
+  assert_int_array_zero_terminated (config, garray);
 
   g_array_set_size (garray, 5);
   g_assert_cmpuint (garray->len, ==, 5);
+  assert_int_array_zero_terminated (config, garray);
 
-  for (i = 0; i < 5; i++)
+  if (config->clear_)
-    g_assert_cmpint (g_array_index (garray, gint, i), ==, 0);
+    for (i = 0; i < 5; i++)
+      g_assert_cmpint (g_array_index (garray, gint, i), ==, 0);
 
   g_array_unref (garray);
 }
@@ -97,16 +141,20 @@ array_new_zero_terminated (void)
   g_free (out_str);
 }
 
+/* Check that g_array_append_val() works correctly for various #GArray
+ * configurations. */
 static void
-array_append (void)
+array_append_val (gconstpointer test_data)
 {
+  const ArrayTestData *config = test_data;
   GArray *garray;
   gint i;
   gint *segment;
 
-  garray = g_array_new (FALSE, FALSE, sizeof (gint));
+  garray = g_array_new (config->zero_terminated, config->clear_, sizeof (gint));
   for (i = 0; i < 10000; i++)
     g_array_append_val (garray, i);
+  assert_int_array_zero_terminated (config, garray);
 
   for (i = 0; i < 10000; i++)
     g_assert_cmpint (g_array_index (garray, gint, i), ==, i);
@@ -114,18 +162,25 @@ array_append (void)
   segment = (gint*)g_array_free (garray, FALSE);
   for (i = 0; i < 10000; i++)
     g_assert_cmpint (segment[i], ==, i);
+  if (config->zero_terminated)
+    g_assert_cmpint (segment[10000], ==, 0);
+
   g_free (segment);
 }
 
+/* Check that g_array_prepend_val() works correctly for various #GArray
+ * configurations. */
 static void
-array_prepend (void)
+array_prepend_val (gconstpointer test_data)
 {
+  const ArrayTestData *config = test_data;
   GArray *garray;
   gint i;
 
-  garray = g_array_new (FALSE, FALSE, sizeof (gint));
+  garray = g_array_new (config->zero_terminated, config->clear_, sizeof (gint));
   for (i = 0; i < 100; i++)
     g_array_prepend_val (garray, i);
+  assert_int_array_zero_terminated (config, garray);
 
   for (i = 0; i < 100; i++)
     g_assert_cmpint (g_array_index (garray, gint, i), ==, (100 - i - 1));
@@ -133,16 +188,141 @@ array_prepend (void)
   g_array_free (garray, TRUE);
 }
 
+/* Test that g_array_prepend_vals() works correctly with various array
+ * configurations. */
 static void
-array_remove (void)
+array_prepend_vals (gconstpointer test_data)
 {
+  const ArrayTestData *config = test_data;
+  GArray *garray, *garray_out;
+  const gint vals[] = { 0, 1, 2, 3, 4 };
+  const gint expected_vals1[] = { 0, 1 };
+  const gint expected_vals2[] = { 2, 0, 1 };
+  const gint expected_vals3[] = { 3, 4, 2, 0, 1 };
+
+  /* Set up an array. */
+  garray = g_array_new (config->zero_terminated, config->clear_, sizeof (gint));
+  assert_int_array_zero_terminated (config, garray);
+
+  /* Prepend several values to an empty array. */
+  garray_out = g_array_prepend_vals (garray, vals, 2);
+  g_assert_true (garray == garray_out);
+  assert_int_array_equal (garray, expected_vals1, G_N_ELEMENTS (expected_vals1));
+  assert_int_array_zero_terminated (config, garray);
+
+  /* Prepend a single value. */
+  garray_out = g_array_prepend_vals (garray, vals + 2, 1);
+  g_assert_true (garray == garray_out);
+  assert_int_array_equal (garray, expected_vals2, G_N_ELEMENTS (expected_vals2));
+  assert_int_array_zero_terminated (config, garray);
+
+  /* Prepend several values to a non-empty array. */
+  garray_out = g_array_prepend_vals (garray, vals + 3, 2);
+  g_assert_true (garray == garray_out);
+  assert_int_array_equal (garray, expected_vals3, G_N_ELEMENTS (expected_vals3));
+  assert_int_array_zero_terminated (config, garray);
+
+  /* Prepend no values. */
+  garray_out = g_array_prepend_vals (garray, vals, 0);
+  g_assert_true (garray == garray_out);
+  assert_int_array_equal (garray, expected_vals3, G_N_ELEMENTS (expected_vals3));
+  assert_int_array_zero_terminated (config, garray);
+
+  /* Prepend no values with %NULL data. */
+  garray_out = g_array_prepend_vals (garray, NULL, 0);
+  g_assert_true (garray == garray_out);
+  assert_int_array_equal (garray, expected_vals3, G_N_ELEMENTS (expected_vals3));
+  assert_int_array_zero_terminated (config, garray);
+
+  g_array_free (garray, TRUE);
+}
+
+/* Test that g_array_insert_vals() works correctly with various array
+ * configurations. */
+static void
+array_insert_vals (gconstpointer test_data)
+{
+  const ArrayTestData *config = test_data;
+  GArray *garray, *garray_out;
+  gsize i;
+  const gint vals[] = { 0, 1, 2, 3, 4, 5, 6, 7 };
+  const gint expected_vals1[] = { 0, 1 };
+  const gint expected_vals2[] = { 0, 2, 3, 1 };
+  const gint expected_vals3[] = { 0, 2, 3, 1, 4 };
+  const gint expected_vals4[] = { 5, 0, 2, 3, 1, 4 };
+  const gint expected_vals5[] = { 5, 0, 2, 3, 1, 4, 0, 0, 0, 0, 6, 7 };
+
+  /* Set up an array. */
+  garray = g_array_new (config->zero_terminated, config->clear_, sizeof (gint));
+  assert_int_array_zero_terminated (config, garray);
+
+  /* Insert several values at the beginning. */
+  garray_out = g_array_insert_vals (garray, 0, vals, 2);
+  g_assert_true (garray == garray_out);
+  assert_int_array_equal (garray, expected_vals1, G_N_ELEMENTS (expected_vals1));
+  assert_int_array_zero_terminated (config, garray);
+
+  /* Insert some more part-way through. */
+  garray_out = g_array_insert_vals (garray, 1, vals + 2, 2);
+  g_assert_true (garray == garray_out);
+  assert_int_array_equal (garray, expected_vals2, G_N_ELEMENTS (expected_vals2));
+  assert_int_array_zero_terminated (config, garray);
+
+  /* And at the end. */
+  garray_out = g_array_insert_vals (garray, garray->len, vals + 4, 1);
+  g_assert_true (garray == garray_out);
+  assert_int_array_equal (garray, expected_vals3, G_N_ELEMENTS (expected_vals3));
+  assert_int_array_zero_terminated (config, garray);
+
+  /* Then back at the beginning again. */
+  garray_out = g_array_insert_vals (garray, 0, vals + 5, 1);
+  g_assert_true (garray == garray_out);
+  assert_int_array_equal (garray, expected_vals4, G_N_ELEMENTS (expected_vals4));
+  assert_int_array_zero_terminated (config, garray);
+
+  /* Insert zero elements. */
+  garray_out = g_array_insert_vals (garray, 0, vals, 0);
+  g_assert_true (garray == garray_out);
+  assert_int_array_equal (garray, expected_vals4, G_N_ELEMENTS (expected_vals4));
+  assert_int_array_zero_terminated (config, garray);
+
+  /* Insert zero elements with a %NULL pointer. */
+  garray_out = g_array_insert_vals (garray, 0, NULL, 0);
+  g_assert_true (garray == garray_out);
+  assert_int_array_equal (garray, expected_vals4, G_N_ELEMENTS (expected_vals4));
+  assert_int_array_zero_terminated (config, garray);
+
+  /* Insert some elements off the end of the array. The behaviour here depends
+   * on whether the array clears entries. */
+  garray_out = g_array_insert_vals (garray, garray->len + 4, vals + 6, 2);
+  g_assert_true (garray == garray_out);
+
+  g_assert_cmpuint (garray->len, ==, G_N_ELEMENTS (expected_vals5));
+  for (i = 0; i < G_N_ELEMENTS (expected_vals5); i++)
+    {
+      if (config->clear_ || i < 6 || i > 9)
+        g_assert_cmpint (g_array_index (garray, gint, i), ==, expected_vals5[i]);
+    }
+
+  assert_int_array_zero_terminated (config, garray);
+
+  g_array_free (garray, TRUE);
+}
+
+/* Check that g_array_remove_index() works correctly for various #GArray
+ * configurations. */
+static void
+array_remove_index (gconstpointer test_data)
+{
+  const ArrayTestData *config = test_data;
   GArray *garray;
   gint i;
   gint prev, cur;
 
-  garray = g_array_new (FALSE, FALSE, sizeof (gint));
+  garray = g_array_new (config->zero_terminated, config->clear_, sizeof (gint));
   for (i = 0; i < 100; i++)
     g_array_append_val (garray, i);
+  assert_int_array_zero_terminated (config, garray);
 
   g_assert_cmpint (garray->len, ==, 100);
 
@@ -152,6 +332,7 @@ array_remove (void)
   g_array_remove_index (garray, 57);
 
   g_assert_cmpint (garray->len, ==, 96);
+  assert_int_array_zero_terminated (config, garray);
 
   prev = -1;
   for (i = 0; i < garray->len; i++)
@@ -165,18 +346,22 @@ array_remove (void)
   g_array_free (garray, TRUE);
 }
 
+/* Check that g_array_remove_index_fast() works correctly for various #GArray
+ * configurations. */
 static void
-array_remove_fast (void)
+array_remove_index_fast (gconstpointer test_data)
 {
+  const ArrayTestData *config = test_data;
   GArray *garray;
   gint i;
   gint prev, cur;
 
-  garray = g_array_new (FALSE, FALSE, sizeof (gint));
+  garray = g_array_new (config->zero_terminated, config->clear_, sizeof (gint));
   for (i = 0; i < 100; i++)
     g_array_append_val (garray, i);
 
   g_assert_cmpint (garray->len, ==, 100);
+  assert_int_array_zero_terminated (config, garray);
 
   g_array_remove_index_fast (garray, 1);
   g_array_remove_index_fast (garray, 3);
@@ -184,6 +369,7 @@ array_remove_fast (void)
   g_array_remove_index_fast (garray, 57);
 
   g_assert_cmpint (garray->len, ==, 96);
+  assert_int_array_zero_terminated (config, garray);
 
   prev = -1;
   for (i = 0; i < garray->len; i++)
@@ -200,22 +386,27 @@ array_remove_fast (void)
   g_array_free (garray, TRUE);
 }
 
+/* Check that g_array_remove_range() works correctly for various #GArray
+ * configurations. */
 static void
-array_remove_range (void)
+array_remove_range (gconstpointer test_data)
 {
+  const ArrayTestData *config = test_data;
   GArray *garray;
   gint i;
   gint prev, cur;
 
-  garray = g_array_new (FALSE, FALSE, sizeof (gint));
+  garray = g_array_new (config->zero_terminated, config->clear_, sizeof (gint));
   for (i = 0; i < 100; i++)
     g_array_append_val (garray, i);
 
   g_assert_cmpint (garray->len, ==, 100);
+  assert_int_array_zero_terminated (config, garray);
 
   g_array_remove_range (garray, 31, 4);
 
   g_assert_cmpint (garray->len, ==, 96);
+  assert_int_array_zero_terminated (config, garray);
 
   prev = -1;
   for (i = 0; i < garray->len; i++)
@@ -228,8 +419,15 @@ array_remove_range (void)
 
   /* Ensure the entire array can be cleared, even when empty. */
   g_array_remove_range (garray, 0, garray->len);
+
+  g_assert_cmpint (garray->len, ==, 0);
+  assert_int_array_zero_terminated (config, garray);
+
   g_array_remove_range (garray, 0, garray->len);
 
+  g_assert_cmpint (garray->len, ==, 0);
+  assert_int_array_zero_terminated (config, garray);
+
   g_array_free (garray, TRUE);
 }
 
@@ -277,20 +475,27 @@ int_compare_data (gconstpointer p1, gconstpointer p2, gpointer data)
 
   return *i1 - *i2;
 }
+
+/* Check that g_array_sort() works correctly for various #GArray
+ * configurations. */
 static void
-array_sort (void)
+array_sort (gconstpointer test_data)
 {
+  const ArrayTestData *config = test_data;
   GArray *garray;
   gint i;
   gint prev, cur;
 
-  garray = g_array_new (FALSE, FALSE, sizeof (gint));
+  garray = g_array_new (config->zero_terminated, config->clear_, sizeof (gint));
   for (i = 0; i < 10000; i++)
     {
       cur = g_random_int_range (0, 10000);
       g_array_append_val (garray, cur);
     }
-  g_array_sort (garray,  int_compare);
+  assert_int_array_zero_terminated (config, garray);
+
+  g_array_sort (garray, int_compare);
+  assert_int_array_zero_terminated (config, garray);
 
   prev = -1;
   for (i = 0; i < garray->len; i++)
@@ -303,20 +508,26 @@ array_sort (void)
   g_array_free (garray, TRUE);
 }
 
+/* Check that g_array_sort_with_data() works correctly for various #GArray
+ * configurations. */
 static void
-array_sort_with_data (void)
+array_sort_with_data (gconstpointer test_data)
 {
+  const ArrayTestData *config = test_data;
   GArray *garray;
   gint i;
   gint prev, cur;
 
-  garray = g_array_new (FALSE, FALSE, sizeof (gint));
+  garray = g_array_new (config->zero_terminated, config->clear_, sizeof (gint));
   for (i = 0; i < 10000; i++)
     {
       cur = g_random_int_range (0, 10000);
       g_array_append_val (garray, cur);
     }
+  assert_int_array_zero_terminated (config, garray);
+
   g_array_sort_with_data (garray, int_compare_data, NULL);
+  assert_int_array_zero_terminated (config, garray);
 
   prev = -1;
   for (i = 0; i < garray->len; i++)
@@ -993,27 +1204,59 @@ byte_array_free_to_bytes (void)
 
   g_bytes_unref (bytes);
 }
+
+static void
+add_array_test (const gchar         *test_path,
+                const ArrayTestData *config,
+                GTestDataFunc        test_func)
+{
+  gchar *test_name = NULL;
+
+  test_name = g_strdup_printf ("%s/%s-%s",
+                               test_path,
+                               config->zero_terminated ? "zero-terminated" : "non-zero-terminated",
+                               config->clear_ ? "clear" : "no-clear");
+  g_test_add_data_func (test_name, config, test_func);
+  g_free (test_name);
+}
+
 int
 main (int argc, char *argv[])
 {
+  /* Test all possible combinations of g_array_new() parameters. */
+  const ArrayTestData array_configurations[] =
+    {
+      { FALSE, FALSE },
+      { FALSE, TRUE },
+      { TRUE, FALSE },
+      { TRUE, TRUE },
+    };
+  gsize i;
+
   g_test_init (&argc, &argv, NULL);
 
   g_test_bug_base ("https://bugzilla.gnome.org/");
 
   /* array tests */
-  g_test_add_func ("/array/new/cleared", array_new_cleared);
-  g_test_add_func ("/array/new/sized-cleared", array_new_sized_cleared);
   g_test_add_func ("/array/new/zero-terminated", array_new_zero_terminated);
-  g_test_add_func ("/array/append", array_append);
-  g_test_add_func ("/array/prepend", array_prepend);
-  g_test_add_func ("/array/remove", array_remove);
-  g_test_add_func ("/array/remove-fast", array_remove_fast);
-  g_test_add_func ("/array/remove-range", array_remove_range);
   g_test_add_func ("/array/ref-count", array_ref_count);
-  g_test_add_func ("/array/sort", array_sort);
-  g_test_add_func ("/array/sort-with-data", array_sort_with_data);
   g_test_add_func ("/array/clear-func", array_clear_func);
 
+  for (i = 0; i < G_N_ELEMENTS (array_configurations); i++)
+    {
+      add_array_test ("/array/set-size", &array_configurations[i], array_set_size);
+      add_array_test ("/array/set-size/sized", &array_configurations[i], array_set_size_sized);
+      add_array_test ("/array/append-val", &array_configurations[i], array_append_val);
+      add_array_test ("/array/prepend-val", &array_configurations[i], array_prepend_val);
+      add_array_test ("/array/prepend-vals", &array_configurations[i], array_prepend_vals);
+      add_array_test ("/array/insert-vals", &array_configurations[i], array_insert_vals);
+      add_array_test ("/array/remove-index", &array_configurations[i], array_remove_index);
+      add_array_test ("/array/remove-index-fast", &array_configurations[i], array_remove_index_fast);
+      add_array_test ("/array/remove-range", &array_configurations[i], array_remove_range);
+      add_array_test ("/array/sort", &array_configurations[i], array_sort);
+      add_array_test ("/array/sort-with-data", &array_configurations[i], array_sort_with_data);
+    }
+
   /* pointer arrays */
   g_test_add_func ("/pointerarray/add", pointer_array_add);
   g_test_add_func ("/pointerarray/insert", pointer_array_insert);