gqsort: Add g_sort_array() and deprecate g_qsort_with_data()

The latter only accepts a `gint` as the number of elements in the array,
which means that its use in `GArray` (and related array implementations)
truncates at least half the potential array size.

So, introduce a replacement for it which uses `size_t` for the number of
elements. This is inline with what `qsort()` (or `qsort_r()`) actually
does. Unfortunately we can’t directly use `qsort_r()` because it’s not
guaranteed to be a stable sort.

This fixes some `-Wsign-conversion` warnings (when building GLib with
that enabled).

Signed-off-by: Philip Withnall <pwithnall@gnome.org>

Helps: #3405
This commit is contained in:
Philip Withnall
2024-06-28 15:22:33 +01:00
parent 616749c1e2
commit b32e1b63ee
5 changed files with 91 additions and 33 deletions

View File

@@ -925,11 +925,11 @@ g_array_sort (GArray *farray,
/* Don't use qsort as we want a guaranteed stable sort */ /* Don't use qsort as we want a guaranteed stable sort */
if (array->len > 0) if (array->len > 0)
g_qsort_with_data (array->data, g_sort_array (array->data,
array->len, array->len,
array->elt_size, array->elt_size,
(GCompareDataFunc)compare_func, (GCompareDataFunc) compare_func,
NULL); NULL);
} }
/** /**
@@ -957,11 +957,11 @@ g_array_sort_with_data (GArray *farray,
g_return_if_fail (array != NULL); g_return_if_fail (array != NULL);
if (array->len > 0) if (array->len > 0)
g_qsort_with_data (array->data, g_sort_array (array->data,
array->len, array->len,
array->elt_size, array->elt_size,
compare_func, compare_func,
user_data); user_data);
} }
/** /**
@@ -2413,11 +2413,11 @@ g_ptr_array_sort (GPtrArray *array,
/* Don't use qsort as we want a guaranteed stable sort */ /* Don't use qsort as we want a guaranteed stable sort */
if (array->len > 0) if (array->len > 0)
g_qsort_with_data (array->pdata, g_sort_array (array->pdata,
array->len, array->len,
sizeof (gpointer), sizeof (gpointer),
(GCompareDataFunc)compare_func, (GCompareDataFunc) compare_func,
NULL); NULL);
} }
/* Please keep this doc-comment in sync with /* Please keep this doc-comment in sync with
@@ -2493,11 +2493,11 @@ g_ptr_array_sort_with_data (GPtrArray *array,
g_return_if_fail (array != NULL); g_return_if_fail (array != NULL);
if (array->len > 0) if (array->len > 0)
g_qsort_with_data (array->pdata, g_sort_array (array->pdata,
array->len, array->len,
sizeof (gpointer), sizeof (gpointer),
compare_func, compare_func,
user_data); user_data);
} }
static inline gint static inline gint

View File

@@ -289,10 +289,14 @@ msort_r (void *b, size_t n, size_t s, GCompareDataFunc cmp, void *arg)
* @compare_func: (scope call): function to compare elements * @compare_func: (scope call): function to compare elements
* @user_data: data to pass to @compare_func * @user_data: data to pass to @compare_func
* *
* This is just like the standard C qsort() function, but * This is just like the standard C [`qsort()`](man:qsort(3)) function, but
* the comparison routine accepts a user data argument. * the comparison routine accepts a user data argument
* (like [`qsort_r()`](man:qsort_r(3))).
* *
* This is guaranteed to be a stable sort since version 2.32. * Unlike `qsort()`, this is guaranteed to be a stable sort (since GLib 2.32).
*
* Deprecated: 2.82: `total_elems` is too small to represent larger arrays; use
* [func@GLib.sort_array] instead
*/ */
void void
g_qsort_with_data (gconstpointer pbase, g_qsort_with_data (gconstpointer pbase,
@@ -301,5 +305,29 @@ g_qsort_with_data (gconstpointer pbase,
GCompareDataFunc compare_func, GCompareDataFunc compare_func,
gpointer user_data) gpointer user_data)
{ {
msort_r ((gpointer)pbase, total_elems, size, compare_func, user_data); g_sort_array (pbase, total_elems, size, compare_func, user_data);
}
/**
* g_sort_array:
* @array: (not nullable) (array length=n_elements): start of array to sort
* @n_elements: number of elements in the array
* @element_size: size of each element
* @compare_func: (scope call): function to compare elements
* @user_data: data to pass to @compare_func
*
* This is just like the standard C [`qsort()`](man:qsort(3)) function, but
* the comparison routine accepts a user data argument
* (like [`qsort_r()`](man:qsort_r(3))).
*
* Unlike `qsort()`, this is guaranteed to be a stable sort.
*/
void
g_sort_array (const void *array,
size_t n_elements,
size_t element_size,
GCompareDataFunc compare_func,
void *user_data)
{
msort_r ((void *) array, n_elements, element_size, compare_func, user_data);
} }

View File

@@ -35,13 +35,20 @@
G_BEGIN_DECLS G_BEGIN_DECLS
GLIB_AVAILABLE_IN_ALL GLIB_DEPRECATED_IN_2_82_FOR(g_sort_array)
void g_qsort_with_data (gconstpointer pbase, void g_qsort_with_data (gconstpointer pbase,
gint total_elems, gint total_elems,
gsize size, gsize size,
GCompareDataFunc compare_func, GCompareDataFunc compare_func,
gpointer user_data); gpointer user_data);
GLIB_AVAILABLE_IN_2_82
void g_sort_array (const void *array,
size_t n_elements,
size_t element_size,
GCompareDataFunc compare_func,
void *user_data);
G_END_DECLS G_END_DECLS
#endif /* __G_QSORT_H__ */ #endif /* __G_QSORT_H__ */

View File

@@ -40,7 +40,7 @@ test_sort_basic (void)
data[i] = g_random_int_range (0, 10000); data[i] = g_random_int_range (0, 10000);
} }
g_qsort_with_data (data, 10000, sizeof (int), int_compare_data, NULL); g_sort_array (data, 10000, sizeof (int), int_compare_data, NULL);
for (i = 1; i < 10000; i++) for (i = 1; i < 10000; i++)
g_assert_cmpint (data[i -1], <=, data[i]); g_assert_cmpint (data[i -1], <=, data[i]);
@@ -63,7 +63,7 @@ test_sort_zero_elements (void)
} }
/* 0 elements is a valid case */ /* 0 elements is a valid case */
g_qsort_with_data (data, 0, sizeof (int), int_compare_data, NULL); g_sort_array (data, 0, sizeof (int), int_compare_data, NULL);
for (i = 0; i < 100; i++) for (i = 0; i < 100; i++)
g_assert_cmpint (data[i], ==, data_copy[i]); g_assert_cmpint (data[i], ==, data_copy[i]);
@@ -105,7 +105,7 @@ test_sort_stable (void)
data[i].i = i; data[i].i = i;
} }
g_qsort_with_data (data, 10000, sizeof (SortItem), item_compare_data, NULL); g_sort_array (data, 10000, sizeof (SortItem), item_compare_data, NULL);
for (i = 1; i < 10000; i++) for (i = 1; i < 10000; i++)
{ {
@@ -129,7 +129,7 @@ test_sort_big (void)
data[i].i = i; data[i].i = i;
} }
g_qsort_with_data (data, 10000, sizeof (BigItem), item_compare_data, NULL); g_sort_array (data, 10000, sizeof (BigItem), item_compare_data, NULL);
for (i = 1; i < 10000; i++) for (i = 1; i < 10000; i++)
{ {
@@ -140,6 +140,28 @@ test_sort_big (void)
g_free (data); g_free (data);
} }
static void
test_sort_deprecated (void)
{
gint *data;
gint i;
data = g_malloc (10000 * sizeof (int));
for (i = 0; i < 10000; i++)
{
data[i] = g_random_int_range (0, 10000);
}
G_GNUC_BEGIN_IGNORE_DEPRECATIONS
g_qsort_with_data (data, 10000, sizeof (int), int_compare_data, NULL);
G_GNUC_END_IGNORE_DEPRECATIONS
for (i = 1; i < 10000; i++)
g_assert_cmpint (data[i -1], <=, data[i]);
g_free (data);
}
int int
main (int argc, char *argv[]) main (int argc, char *argv[])
{ {
@@ -149,6 +171,7 @@ main (int argc, char *argv[])
g_test_add_func ("/sort/zero-elements", test_sort_zero_elements); g_test_add_func ("/sort/zero-elements", test_sort_zero_elements);
g_test_add_func ("/sort/stable", test_sort_stable); g_test_add_func ("/sort/stable", test_sort_stable);
g_test_add_func ("/sort/big", test_sort_big); g_test_add_func ("/sort/big", test_sort_big);
g_test_add_func ("/sort/deprecated", test_sort_deprecated);
return g_test_run (); return g_test_run ();
} }

View File

@@ -364,9 +364,9 @@ g_value_array_sort_with_data (GValueArray *value_array,
g_return_val_if_fail (compare_func != NULL, NULL); g_return_val_if_fail (compare_func != NULL, NULL);
if (value_array->n_values) if (value_array->n_values)
g_qsort_with_data (value_array->values, g_sort_array (value_array->values,
value_array->n_values, value_array->n_values,
sizeof (value_array->values[0]), sizeof (value_array->values[0]),
compare_func, user_data); compare_func, user_data);
return value_array; return value_array;
} }