From 0ffd23cac4f27ae37a67cf562690b6a51990ca4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Tue, 20 Dec 2022 00:14:18 +0100 Subject: [PATCH 1/2] garray: Add g_ptr_array_sort_values[_with_data]() wrappers Historically GPtrArray made possible to compare pointers of pointers values that it holds, however this is inconvenient in most cases as it requires wrapper functions and not friendly castings. So, add two functions that allow to perform the comparisons between the pointer values that a GPtrArray holds following the same syntax that we share everywhere in the codebase. --- docs/reference/glib/glib-sections.txt.in | 2 + glib/garray.c | 84 ++++++++- glib/garray.h | 7 + glib/tests/array-test.c | 218 ++++++++++++++++++++++- 4 files changed, 307 insertions(+), 4 deletions(-) diff --git a/docs/reference/glib/glib-sections.txt.in b/docs/reference/glib/glib-sections.txt.in index ab7453e60..0ec7d77ba 100644 --- a/docs/reference/glib/glib-sections.txt.in +++ b/docs/reference/glib/glib-sections.txt.in @@ -2736,6 +2736,8 @@ g_ptr_array_steal_index g_ptr_array_steal_index_fast g_ptr_array_sort g_ptr_array_sort_with_data +g_ptr_array_sort_values +g_ptr_array_sort_values_with_data g_ptr_array_set_size g_ptr_array_index g_ptr_array_free diff --git a/glib/garray.c b/glib/garray.c index ee4b859fe..42f36b2be 100644 --- a/glib/garray.c +++ b/glib/garray.c @@ -2124,7 +2124,10 @@ g_ptr_array_insert (GPtrArray *array, * * Note that the comparison function for g_ptr_array_sort() doesn't * take the pointers from the array as arguments, it takes pointers to - * the pointers in the array. Here is a full example of usage: + * the pointers in the array. + * + * Use g_ptr_array_sort_with_data() if you want to use normal + * #GCompareFuncs, otherwise here is a full example of use: * * |[ * typedef struct @@ -2181,7 +2184,10 @@ g_ptr_array_sort (GPtrArray *array, * * Note that the comparison function for g_ptr_array_sort_with_data() * doesn't take the pointers from the array as arguments, it takes - * pointers to the pointers in the array. Here is a full example of use: + * pointers to the pointers in the array. + * + * Use g_ptr_array_sort_with_data() if you want to use normal + * #GCompareDataFuncs, otherwise here is a full example of use: * * |[ * typedef enum { SORT_NAME, SORT_SIZE } SortMode; @@ -2245,6 +2251,80 @@ g_ptr_array_sort_with_data (GPtrArray *array, user_data); } +static inline gint +compare_ptr_array_values (gconstpointer a, gconstpointer b, gpointer user_data) +{ + gconstpointer aa = *((gconstpointer *) a); + gconstpointer bb = *((gconstpointer *) b); + GCompareFunc compare_func = user_data; + + return compare_func (aa, bb); +} + +/** + * g_ptr_array_sort_values: + * @array: a #GPtrArray + * @compare_func: a #GCompareFunc comparison function + * + * Sorts the array, using @compare_func which should be a qsort()-style + * comparison function (returns less than zero for first arg is less + * than second arg, zero for equal, greater than zero if first arg is + * greater than second arg). + * + * This is guaranteed to be a stable sort. + * + * Since: 2.76 + */ +void +g_ptr_array_sort_values (GPtrArray *array, + GCompareFunc compare_func) +{ + g_ptr_array_sort_with_data (array, compare_ptr_array_values, compare_func); +} + +typedef struct +{ + GCompareDataFunc compare_func; + gpointer user_data; +} GPtrArraySortValuesData; + +static inline gint +compare_ptr_array_values_with_data (gconstpointer a, + gconstpointer b, + gpointer user_data) +{ + gconstpointer aa = *((gconstpointer *) a); + gconstpointer bb = *((gconstpointer *) b); + GPtrArraySortValuesData *data = user_data; + + return data->compare_func (aa, bb, data->user_data); +} + +/** + * g_ptr_array_sort_values_with_data: + * @array: a #GPtrArray + * @compare_func: a #GCompareDataFunc comparison function + * @user_data: data to pass to @compare_func + * + * Like g_ptr_array_sort_values(), but the comparison function has an extra + * user data argument. + * + * This is guaranteed to be a stable sort. + * + * Since: 2.76 + */ +void +g_ptr_array_sort_values_with_data (GPtrArray *array, + GCompareDataFunc compare_func, + gpointer user_data) +{ + g_ptr_array_sort_with_data (array, compare_ptr_array_values_with_data, + &(GPtrArraySortValuesData){ + .compare_func = compare_func, + .user_data = user_data, + }); +} + /** * g_ptr_array_foreach: * @array: a #GPtrArray diff --git a/glib/garray.h b/glib/garray.h index 5b9c6752a..16f74015e 100644 --- a/glib/garray.h +++ b/glib/garray.h @@ -242,6 +242,13 @@ GLIB_AVAILABLE_IN_ALL void g_ptr_array_sort_with_data (GPtrArray *array, GCompareDataFunc compare_func, gpointer user_data); +GLIB_AVAILABLE_IN_2_76 +void g_ptr_array_sort_values (GPtrArray *array, + GCompareFunc compare_func); +GLIB_AVAILABLE_IN_2_76 +void g_ptr_array_sort_values_with_data (GPtrArray *array, + GCompareDataFunc compare_func, + gpointer user_data); GLIB_AVAILABLE_IN_ALL void g_ptr_array_foreach (GPtrArray *array, GFunc func, diff --git a/glib/tests/array-test.c b/glib/tests/array-test.c index 9826d3e33..257170567 100644 --- a/glib/tests/array-test.c +++ b/glib/tests/array-test.c @@ -1474,13 +1474,25 @@ pointer_array_extend_and_steal (void) g_free (array_test); } +static gint +ptr_compare_values (gconstpointer p1, gconstpointer p2) +{ + return GPOINTER_TO_INT (p1) - GPOINTER_TO_INT (p2); +} + static gint ptr_compare (gconstpointer p1, gconstpointer p2) { gpointer i1 = *(gpointer*)p1; gpointer i2 = *(gpointer*)p2; - return GPOINTER_TO_INT (i1) - GPOINTER_TO_INT (i2); + return ptr_compare_values (i1, i2); +} + +static gint +ptr_compare_values_data (gconstpointer p1, gconstpointer p2, gpointer data) +{ + return GPOINTER_TO_INT (p1) - GPOINTER_TO_INT (p2); } static gint @@ -1489,7 +1501,7 @@ ptr_compare_data (gconstpointer p1, gconstpointer p2, gpointer data) gpointer i1 = *(gpointer*)p1; gpointer i2 = *(gpointer*)p2; - return GPOINTER_TO_INT (i1) - GPOINTER_TO_INT (i2); + return ptr_compare_values_data (i1, i2, data); } static void @@ -1696,6 +1708,204 @@ pointer_array_sort_with_data (void) g_ptr_array_free (gparray, TRUE); } +static void +pointer_array_sort_values (void) +{ + GPtrArray *gparray; + gint i; + gint val; + gint prev, cur; + + gparray = g_ptr_array_new (); + + /* Sort empty array */ + g_ptr_array_sort_values (gparray, ptr_compare_values); + + for (i = 0; i < 10000; i++) + { + val = g_random_int_range (0, 10000); + g_ptr_array_add (gparray, GINT_TO_POINTER (val)); + } + + g_ptr_array_sort_values (gparray, ptr_compare_values); + + prev = -1; + for (i = 0; i < 10000; i++) + { + cur = GPOINTER_TO_INT (g_ptr_array_index (gparray, i)); + g_assert_cmpint (prev, <=, cur); + prev = cur; + } + + g_clear_pointer (&gparray, g_ptr_array_unref); + + gparray = g_ptr_array_new (); + + g_ptr_array_add (gparray, "dddd"); + g_ptr_array_add (gparray, "cccc"); + g_ptr_array_add (gparray, NULL); + g_ptr_array_add (gparray, "bbbb"); + g_ptr_array_add (gparray, "aaaa"); + + g_ptr_array_sort_values (gparray, (GCompareFunc) g_strcmp0); + + i = 0; + g_assert_cmpstr (g_ptr_array_index (gparray, i++), ==, NULL); + g_assert_cmpstr (g_ptr_array_index (gparray, i++), ==, "aaaa"); + g_assert_cmpstr (g_ptr_array_index (gparray, i++), ==, "bbbb"); + g_assert_cmpstr (g_ptr_array_index (gparray, i++), ==, "cccc"); + g_assert_cmpstr (g_ptr_array_index (gparray, i++), ==, "dddd"); + + g_clear_pointer (&gparray, g_ptr_array_unref); +} + +static gint +sort_filelist_values (gconstpointer a, gconstpointer b) +{ + const FileListEntry *entry1 = a; + const FileListEntry *entry2 = b; + + return g_ascii_strcasecmp (entry1->name, entry2->name); +} + +static void +pointer_array_sort_values_example (void) +{ + GPtrArray *file_list = NULL; + FileListEntry *entry; + + file_list = g_ptr_array_new_with_free_func (file_list_entry_free); + + entry = g_new0 (FileListEntry, 1); + entry->name = g_strdup ("README"); + entry->size = 42; + g_ptr_array_add (file_list, g_steal_pointer (&entry)); + + entry = g_new0 (FileListEntry, 1); + entry->name = g_strdup ("empty"); + entry->size = 0; + g_ptr_array_add (file_list, g_steal_pointer (&entry)); + + entry = g_new0 (FileListEntry, 1); + entry->name = g_strdup ("aardvark"); + entry->size = 23; + g_ptr_array_add (file_list, g_steal_pointer (&entry)); + + g_ptr_array_sort_values (file_list, sort_filelist_values); + + g_assert_cmpuint (file_list->len, ==, 3); + entry = g_ptr_array_index (file_list, 0); + g_assert_cmpstr (entry->name, ==, "aardvark"); + entry = g_ptr_array_index (file_list, 1); + g_assert_cmpstr (entry->name, ==, "empty"); + entry = g_ptr_array_index (file_list, 2); + g_assert_cmpstr (entry->name, ==, "README"); + + g_ptr_array_unref (file_list); +} + +static gint +sort_filelist_how_values (gconstpointer a, gconstpointer b, gpointer user_data) +{ + gint order; + const SortMode sort_mode = GPOINTER_TO_INT (user_data); + const FileListEntry *entry1 = a; + const FileListEntry *entry2 = b; + + switch (sort_mode) + { + case SORT_NAME: + order = g_ascii_strcasecmp (entry1->name, entry2->name); + break; + case SORT_SIZE: + order = entry1->size - entry2->size; + break; + default: + order = 0; + break; + } + return order; +} + +static void +pointer_array_sort_values_with_data_example (void) +{ + GPtrArray *file_list = NULL; + FileListEntry *entry; + SortMode sort_mode; + + file_list = g_ptr_array_new_with_free_func (file_list_entry_free); + + entry = g_new0 (FileListEntry, 1); + entry->name = g_strdup ("README"); + entry->size = 42; + g_ptr_array_add (file_list, g_steal_pointer (&entry)); + + entry = g_new0 (FileListEntry, 1); + entry->name = g_strdup ("empty"); + entry->size = 0; + g_ptr_array_add (file_list, g_steal_pointer (&entry)); + + entry = g_new0 (FileListEntry, 1); + entry->name = g_strdup ("aardvark"); + entry->size = 23; + g_ptr_array_add (file_list, g_steal_pointer (&entry)); + + sort_mode = SORT_NAME; + g_ptr_array_sort_values_with_data (file_list, sort_filelist_how_values, + GINT_TO_POINTER (sort_mode)); + + g_assert_cmpuint (file_list->len, ==, 3); + entry = g_ptr_array_index (file_list, 0); + g_assert_cmpstr (entry->name, ==, "aardvark"); + entry = g_ptr_array_index (file_list, 1); + g_assert_cmpstr (entry->name, ==, "empty"); + entry = g_ptr_array_index (file_list, 2); + g_assert_cmpstr (entry->name, ==, "README"); + + sort_mode = SORT_SIZE; + g_ptr_array_sort_values_with_data (file_list, sort_filelist_how_values, + GINT_TO_POINTER (sort_mode)); + + g_assert_cmpuint (file_list->len, ==, 3); + entry = g_ptr_array_index (file_list, 0); + g_assert_cmpstr (entry->name, ==, "empty"); + entry = g_ptr_array_index (file_list, 1); + g_assert_cmpstr (entry->name, ==, "aardvark"); + entry = g_ptr_array_index (file_list, 2); + g_assert_cmpstr (entry->name, ==, "README"); + + g_ptr_array_unref (file_list); +} + +static void +pointer_array_sort_values_with_data (void) +{ + GPtrArray *gparray; + gint i; + gint prev, cur; + + gparray = g_ptr_array_new (); + + /* Sort empty array */ + g_ptr_array_sort_values_with_data (gparray, ptr_compare_values_data, NULL); + + for (i = 0; i < 10000; i++) + g_ptr_array_add (gparray, GINT_TO_POINTER (g_random_int_range (0, 10000))); + + g_ptr_array_sort_values_with_data (gparray, ptr_compare_values_data, NULL); + + prev = -1; + for (i = 0; i < 10000; i++) + { + cur = GPOINTER_TO_INT (g_ptr_array_index (gparray, i)); + g_assert_cmpint (prev, <=, cur); + prev = cur; + } + + g_ptr_array_free (gparray, TRUE); +} + static void pointer_array_find_empty (void) { @@ -2240,6 +2450,10 @@ main (int argc, char *argv[]) g_test_add_func ("/pointerarray/sort/example", pointer_array_sort_example); g_test_add_func ("/pointerarray/sort-with-data", pointer_array_sort_with_data); g_test_add_func ("/pointerarray/sort-with-data/example", pointer_array_sort_with_data_example); + g_test_add_func ("/pointerarray/sort-values", pointer_array_sort_values); + g_test_add_func ("/pointerarray/sort-values/example", pointer_array_sort_values_example); + g_test_add_func ("/pointerarray/sort-values-with-data", pointer_array_sort_values_with_data); + g_test_add_func ("/pointerarray/sort-values-with-data/example", pointer_array_sort_values_with_data_example); g_test_add_func ("/pointerarray/find/empty", pointer_array_find_empty); g_test_add_func ("/pointerarray/find/non-empty", pointer_array_find_non_empty); g_test_add_func ("/pointerarray/remove-range", pointer_array_remove_range); From 861e82efbc9fb9ad2c84ad4cb5671ee19d772a8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Tue, 20 Dec 2022 00:22:05 +0100 Subject: [PATCH 2/2] gio: Use g_ptr_array_sort_values() Cleanup some code using GPtrArray sorting with functions that were taking pointer to pointers arguments. --- gio/gdbus-tool.c | 8 +------- gio/gdbusproxy.c | 9 +-------- gio/glib-compile-schemas.c | 8 ++++---- gio/tests/gdbus-export.c | 6 +++--- gio/tests/gdbus-test-codegen.c | 18 ++++++++---------- gio/tests/glistmodel.c | 18 ++++++------------ 6 files changed, 23 insertions(+), 44 deletions(-) diff --git a/gio/gdbus-tool.c b/gio/gdbus-tool.c index 5f9f9dd5a..2435998d3 100644 --- a/gio/gdbus-tool.c +++ b/gio/gdbus-tool.c @@ -317,12 +317,6 @@ print_paths (GDBusConnection *c, ; } -static gint -ptr_strcmp0 (const gchar **a, const gchar **b) -{ - return g_strcmp0 (*a, *b); -} - static void print_names (GDBusConnection *c, gboolean include_unique_names) @@ -385,7 +379,7 @@ print_names (GDBusConnection *c, g_variant_unref (result); keys = g_hash_table_steal_all_keys (name_set); - g_ptr_array_sort (keys, (GCompareFunc) ptr_strcmp0); + g_ptr_array_sort_values (keys, (GCompareFunc) g_strcmp0); for (guint i = 0; i < keys->len; ++i) { const gchar *name = g_ptr_array_index (keys, i); diff --git a/gio/gdbusproxy.c b/gio/gdbusproxy.c index 292c7b5e1..afc6fe94e 100644 --- a/gio/gdbusproxy.c +++ b/gio/gdbusproxy.c @@ -631,13 +631,6 @@ g_dbus_proxy_init (GDBusProxy *proxy) /* ---------------------------------------------------------------------------------------------------- */ -static gint -property_name_sort_func (const gchar **a, - const gchar **b) -{ - return g_strcmp0 (*a, *b); -} - /** * g_dbus_proxy_get_cached_property_names: * @proxy: A #GDBusProxy. @@ -672,7 +665,7 @@ g_dbus_proxy_get_cached_property_names (GDBusProxy *proxy) g_hash_table_iter_init (&iter, proxy->priv->properties); while (g_hash_table_iter_next (&iter, (gpointer) &key, NULL)) g_ptr_array_add (p, g_strdup (key)); - g_ptr_array_sort (p, (GCompareFunc) property_name_sort_func); + g_ptr_array_sort_values (p, (GCompareFunc) g_strcmp0); g_ptr_array_add (p, NULL); names = (gchar **) g_ptr_array_free (p, FALSE); diff --git a/gio/glib-compile-schemas.c b/gio/glib-compile-schemas.c index 326658a88..2352f22df 100644 --- a/gio/glib-compile-schemas.c +++ b/gio/glib-compile-schemas.c @@ -1869,8 +1869,8 @@ static gint compare_strings (gconstpointer a, gconstpointer b) { - gchar *one = *(gchar **) a; - gchar *two = *(gchar **) b; + const gchar *one = a; + const gchar *two = a; gint cmp; cmp = g_str_has_suffix (two, ".enums.xml") - @@ -2278,10 +2278,10 @@ main (int argc, char **argv) retval = 0; goto done; } - g_ptr_array_sort (files, compare_strings); + g_ptr_array_sort_values (files, compare_strings); g_ptr_array_add (files, NULL); - g_ptr_array_sort (overrides, compare_strings); + g_ptr_array_sort_values (overrides, compare_strings); g_ptr_array_add (overrides, NULL); schema_files = (char **) g_ptr_array_free (files, FALSE); diff --git a/gio/tests/gdbus-export.c b/gio/tests/gdbus-export.c index ab8a371f5..2080ebe12 100644 --- a/gio/tests/gdbus-export.c +++ b/gio/tests/gdbus-export.c @@ -347,8 +347,8 @@ static gint compare_strings (gconstpointer a, gconstpointer b) { - const gchar *sa = *(const gchar **) a; - const gchar *sb = *(const gchar **) b; + const gchar *sa = a; + const gchar *sb = b; /* Array terminator must sort last */ if (sa == NULL) @@ -413,7 +413,7 @@ get_nodes_at (GDBusConnection *c, g_dbus_node_info_unref (node_info); /* Nodes are semantically unordered; sort array so tests can rely on order */ - g_ptr_array_sort (p, compare_strings); + g_ptr_array_sort_values (p, compare_strings); return (gchar **) g_ptr_array_free (p, FALSE); } diff --git a/gio/tests/gdbus-test-codegen.c b/gio/tests/gdbus-test-codegen.c index 0a180b8fc..3fa83e761 100644 --- a/gio/tests/gdbus-test-codegen.c +++ b/gio/tests/gdbus-test-codegen.c @@ -1551,12 +1551,6 @@ typedef struct guint num_interface_removed_signals; } OMData; -static gint -my_pstrcmp (const gchar **a, const gchar **b) -{ - return g_strcmp0 (*a, *b); -} - static void om_check_interfaces_added (const gchar *signal_name, GVariant *parameters, @@ -1597,8 +1591,10 @@ om_check_interfaces_added (const gchar *signal_name, g_ptr_array_add (interfaces_in_message, (gpointer) iface_name); } g_assert_cmpint (interfaces_in_message->len, ==, interfaces->len); - g_ptr_array_sort (interfaces, (GCompareFunc) my_pstrcmp); - g_ptr_array_sort (interfaces_in_message, (GCompareFunc) my_pstrcmp); + G_GNUC_BEGIN_IGNORE_DEPRECATIONS + g_ptr_array_sort_values (interfaces, (GCompareFunc) g_strcmp0); + g_ptr_array_sort_values (interfaces_in_message, (GCompareFunc) g_strcmp0); + G_GNUC_END_IGNORE_DEPRECATIONS for (n = 0; n < interfaces->len; n++) g_assert_cmpstr (interfaces->pdata[n], ==, interfaces_in_message->pdata[n]); g_ptr_array_unref (interfaces_in_message); @@ -1646,8 +1642,10 @@ om_check_interfaces_removed (const gchar *signal_name, g_ptr_array_add (interfaces_in_message, (gpointer) iface_name); } g_assert_cmpint (interfaces_in_message->len, ==, interfaces->len); - g_ptr_array_sort (interfaces, (GCompareFunc) my_pstrcmp); - g_ptr_array_sort (interfaces_in_message, (GCompareFunc) my_pstrcmp); + G_GNUC_BEGIN_IGNORE_DEPRECATIONS + g_ptr_array_sort_values (interfaces, (GCompareFunc) g_strcmp0); + g_ptr_array_sort_values (interfaces_in_message, (GCompareFunc) g_strcmp0); + G_GNUC_END_IGNORE_DEPRECATIONS for (n = 0; n < interfaces->len; n++) g_assert_cmpstr (interfaces->pdata[n], ==, interfaces_in_message->pdata[n]); g_ptr_array_unref (interfaces_in_message); diff --git a/gio/tests/glistmodel.c b/gio/tests/glistmodel.c index 049c8e4a8..cd6ae91e2 100644 --- a/gio/tests/glistmodel.c +++ b/gio/tests/glistmodel.c @@ -572,13 +572,7 @@ test_store_splice_wrong_type (void) } static gint -ptr_array_cmp_action_by_name (GAction **a, GAction **b) -{ - return g_strcmp0 (g_action_get_name (*a), g_action_get_name (*b)); -} - -static gint -list_model_cmp_action_by_name (GAction *a, GAction *b, gpointer user_data) +cmp_action_by_name (GAction *a, GAction *b, gpointer user_data) { return g_strcmp0 (g_action_get_name (a), g_action_get_name (b)); } @@ -606,16 +600,16 @@ test_store_sort (void) g_ptr_array_add (array, g_simple_action_new ("1", NULL)); /* Sort an empty list */ - g_list_store_sort (store, (GCompareDataFunc)list_model_cmp_action_by_name, NULL); + g_list_store_sort (store, (GCompareDataFunc) cmp_action_by_name, NULL); /* Add all */ g_list_store_splice (store, 0, 0, array->pdata, array->len); g_assert_true (model_array_equal (model, array)); /* Sort both and check if the result is the same */ - g_ptr_array_sort (array, (GCompareFunc)ptr_array_cmp_action_by_name); + g_ptr_array_sort_values (array, (GCompareFunc)cmp_action_by_name); g_assert_false (model_array_equal (model, array)); - g_list_store_sort (store, (GCompareDataFunc)list_model_cmp_action_by_name, NULL); + g_list_store_sort (store, (GCompareDataFunc) cmp_action_by_name, NULL); g_assert_true (model_array_equal (model, array)); g_ptr_array_unref (array); @@ -763,7 +757,7 @@ test_store_signal_items_changed (void) /* Sort the list */ expect_items_changed (&expected, 0, 2, 2); g_list_store_sort (store, - (GCompareDataFunc)list_model_cmp_action_by_name, + (GCompareDataFunc) cmp_action_by_name, NULL); g_assert_true (expected.called); g_assert_false (expected.notified); @@ -773,7 +767,7 @@ test_store_signal_items_changed (void) item = g_simple_action_new ("3", NULL); g_list_store_insert_sorted (store, item, - (GCompareDataFunc)list_model_cmp_action_by_name, + (GCompareDataFunc) cmp_action_by_name, NULL); g_object_unref (item); g_assert_true (expected.called);