From 7e3ec9a8ae2f815f0a8e513a5e64aa4b4b5d3af6 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 2 May 2024 14:33:02 +0100 Subject: [PATCH 1/6] girepository: Keep an ordered list of the loaded typelibs There are various places where the set of typelibs is iterated over or returned in an ordered way. In order to keep results deterministic and reproducible, we need to keep this set ordered. Keep a `GPtrArray` of the typelibs (one for fully-loaded ones and one for lazy ones) alongside the existing hash tables. This will be used for iteration in the next few commits. Signed-off-by: Philip Withnall Helps: #3303 --- girepository/girepository.c | 65 +++++++++++++++++++++++++++++++++++-- 1 file changed, 63 insertions(+), 2 deletions(-) diff --git a/girepository/girepository.c b/girepository/girepository.c index 24d83ca25..547913192 100644 --- a/girepository/girepository.c +++ b/girepository/girepository.c @@ -65,6 +65,23 @@ * The environment variable takes precedence over the default search path * and the [method@GIRepository.Repository.prepend_search_path] calls. * + * ### Namespace ordering + * + * In situations where namespaces may be searched in order, or returned in a + * list, the namespaces will be returned in alphabetical order, with all fully + * loaded namespaces being returned before any lazily loaded ones (those loaded + * with `GI_REPOSITORY_LOAD_FLAG_LAZY`). This allows for deterministic and + * reproducible results. + * + * Similarly, if a symbol (such as a `GType` or error domain) is being searched + * for in the set of loaded namespaces, the namespaces will be searched in that + * order. In particular, this means that a symbol which exists in two namespaces + * will always be returned from the alphabetically-higher namespace. This should + * only happen in the case of `Gio` and `GioUnix`/`GioWin32`, which all refer to + * the same `.so` file and expose overlapping sets of symbols. Symbols should + * always end up being resolved to `GioUnix` or `GioWin32` if they are platform + * dependent, rather than `Gio` itself. + * * Since: 2.80 */ @@ -98,8 +115,14 @@ struct _GIRepository GPtrArray *typelib_search_path; /* (element-type filename) (owned) */ GPtrArray *library_paths; /* (element-type filename) (owned) */ + /* Certain operations require iterating over the typelibs and the iteration + * order may affect the results. So keep an ordered list of the typelibs, + * alongside the hash table which keep the canonical strong reference to them. */ GHashTable *typelibs; /* (string) namespace -> GITypelib */ + GPtrArray *ordered_typelibs; /* (element-type unowned GITypelib) (owned) (not nullable) */ GHashTable *lazy_typelibs; /* (string) namespace-version -> GITypelib */ + GPtrArray *ordered_lazy_typelibs; /* (element-type unowned GITypelib) (owned) (not nullable) */ + GHashTable *info_by_gtype; /* GType -> GIBaseInfo */ GHashTable *info_by_error_domain; /* GQuark -> GIBaseInfo */ GHashTable *interfaces_for_gtype; /* GType -> GTypeInterfaceCache */ @@ -187,10 +210,13 @@ gi_repository_init (GIRepository *repository) = g_hash_table_new_full (g_str_hash, g_str_equal, (GDestroyNotify) g_free, (GDestroyNotify) gi_typelib_unref); + repository->ordered_typelibs = g_ptr_array_new_with_free_func (NULL); repository->lazy_typelibs = g_hash_table_new_full (g_str_hash, g_str_equal, (GDestroyNotify) g_free, (GDestroyNotify) gi_typelib_unref); + repository->ordered_lazy_typelibs = g_ptr_array_new_with_free_func (NULL); + repository->info_by_gtype = g_hash_table_new_full (g_direct_hash, g_direct_equal, (GDestroyNotify) NULL, @@ -212,7 +238,10 @@ gi_repository_finalize (GObject *object) GIRepository *repository = GI_REPOSITORY (object); g_hash_table_destroy (repository->typelibs); + g_ptr_array_unref (repository->ordered_typelibs); g_hash_table_destroy (repository->lazy_typelibs); + g_ptr_array_unref (repository->ordered_lazy_typelibs); + g_hash_table_destroy (repository->info_by_gtype); g_hash_table_destroy (repository->info_by_error_domain); g_hash_table_destroy (repository->interfaces_for_gtype); @@ -497,6 +526,29 @@ load_dependencies_recurse (GIRepository *repository, return TRUE; } +/* Sort typelibs by namespace. The main requirement here is just to make iteration + * deterministic, otherwise results can change as a lot of the code here would + * just iterate over a `GHashTable`. + * + * A sub-requirement of this is that namespaces are sorted such that if a GType + * or symbol is found in multiple namespaces where one is a prefix of the other, + * the longest namespace wins. In practice, this only happens in + * Gio/GioUnix/GioWin32, as all three of those namespaces refer to the same + * `.so` file and overlapping sets of the same symbols, but we want the platform + * specific namespace to be returned in preference to anything else (even though + * either namespace is valid). + * See https://gitlab.gnome.org/GNOME/glib/-/issues/3303 */ +static int +sort_typelibs_cb (const void *a, + const void *b) +{ + GITypelib *typelib_a = *(GITypelib **) a; + GITypelib *typelib_b = *(GITypelib **) b; + + return strcmp (gi_typelib_get_namespace (typelib_a), + gi_typelib_get_namespace (typelib_b)); +} + static const char * register_internal (GIRepository *repository, const char *source, @@ -521,6 +573,8 @@ register_internal (GIRepository *repository, namespace)); g_hash_table_insert (repository->lazy_typelibs, build_typelib_key (namespace, source), gi_typelib_ref (typelib)); + g_ptr_array_add (repository->ordered_lazy_typelibs, typelib); + g_ptr_array_sort (repository->ordered_lazy_typelibs, sort_typelibs_cb); } else { @@ -535,13 +589,20 @@ register_internal (GIRepository *repository, if (g_hash_table_lookup_extended (repository->lazy_typelibs, namespace, (gpointer)&key, &value)) - g_hash_table_remove (repository->lazy_typelibs, key); + { + g_hash_table_remove (repository->lazy_typelibs, key); + g_ptr_array_remove (repository->ordered_lazy_typelibs, typelib); + } else - key = build_typelib_key (namespace, source); + { + key = build_typelib_key (namespace, source); + } g_hash_table_insert (repository->typelibs, g_steal_pointer (&key), gi_typelib_ref (typelib)); + g_ptr_array_add (repository->ordered_typelibs, typelib); + g_ptr_array_sort (repository->ordered_typelibs, sort_typelibs_cb); } /* These types might be resolved now, clear the cache */ From 04bdf50c68e15a5cd9ba46de23d0b5567ce1403b Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 2 May 2024 14:36:36 +0100 Subject: [PATCH 2/6] girepository: Fix a typo in a code comment Signed-off-by: Philip Withnall --- girepository/girepository.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/girepository/girepository.c b/girepository/girepository.c index 547913192..2c65bbcaa 100644 --- a/girepository/girepository.c +++ b/girepository/girepository.c @@ -1015,7 +1015,7 @@ gi_repository_find_by_gtype (GIRepository *repository, if (entry == NULL) entry = find_by_gtype (repository->lazy_typelibs, &data, TRUE); - /* Not ever class library necessarily specifies a correct c_prefix, + /* Not every class library necessarily specifies a correct c_prefix, * so take a second pass. This time we will try a global lookup, * ignoring prefixes. * See http://bugzilla.gnome.org/show_bug.cgi?id=564016 From 25b7ecf895b968e9275ecd527e68d648a59ed52a Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 2 May 2024 15:59:46 +0100 Subject: [PATCH 3/6] gitypelib: Fix iterating through typelib prefixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The iteration code used `g_string_overwrite_len()` to try and simplify buffer allocation and growth, but seemingly forgot to handle the fact that it doesn’t nul-terminate what it overwrites: the method is intended to be used to splice bits into longer strings, not to overwrite an entire nul-terminated string. This meant that when iterating over a comma-separated `c_prefix` like `GUnix,G`, on the second iteration `g_string_overwrite_len()` would be used to write `G` into index 0 of the already-set `GUnix` string in the buffer, leading to the first iteration happening all over again and the `G` prefix being ignored. This led to symbols failing to be matched to the `GioUnix` typelib, even though they should have been. This will be checked by a test in the following commit. Signed-off-by: Philip Withnall Helps: #3303 --- girepository/gitypelib.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/girepository/gitypelib.c b/girepository/gitypelib.c index 3c88a79e6..57411a86b 100644 --- a/girepository/gitypelib.c +++ b/girepository/gitypelib.c @@ -316,7 +316,8 @@ strsplit_iter_next (StrSplitIter *iter, } else { - g_string_overwrite_len (&iter->buf, 0, s, (gssize)len); + g_string_overwrite_len (&iter->buf, 0, s, (gssize)len + 1); + iter->buf.str[len] = '\0'; *out_val = iter->buf.str; } return TRUE; From 48988a4098e1c128e591c04b7de8785f10db4e45 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 2 May 2024 16:03:24 +0100 Subject: [PATCH 4/6] girepository: Make gi_repository_find_by_gtype() deterministic When faced with a `GType` which is present in multiple typelibs, the old implementation was not deterministic, as it iterated over a hash table of typelibs. The iteration order of a hash table is not deterministic. Use the new `ordered_typelibs` and `ordered_lazy_typelibs` arrays to iterate instead, making the order deterministic. Add a unit test to check this. In particular, to check that symbols which are present in both `Gio` and `GioUnix` are correctly resolved as being from `GioUnix`. Signed-off-by: Philip Withnall Helps: #3303 --- girepository/girepository.c | 45 ++++++++--------- girepository/introspection/meson.build | 2 + girepository/tests/meson.build | 8 ++- girepository/tests/repository.c | 68 ++++++++++++++++++++++++++ girepository/tests/test-common.h | 5 ++ 5 files changed, 103 insertions(+), 25 deletions(-) diff --git a/girepository/girepository.c b/girepository/girepository.c index 2c65bbcaa..e05012b4e 100644 --- a/girepository/girepository.c +++ b/girepository/girepository.c @@ -931,32 +931,29 @@ gi_repository_get_info (GIRepository *repository, NULL, typelib, entry->offset); } -typedef struct { - const char *gtype_name; - GITypelib *result_typelib; -} FindByGTypeData; - static DirEntry * -find_by_gtype (GHashTable *table, FindByGTypeData *data, gboolean check_prefix) +find_by_gtype (GPtrArray *ordered_table, + const char *gtype_name, + gboolean check_prefix, + GITypelib **out_result_typelib) { - GHashTableIter iter; - gpointer key, value; - DirEntry *ret; - - g_hash_table_iter_init (&iter, table); - while (g_hash_table_iter_next (&iter, &key, &value)) + /* Search in reverse order as the longest namespaces will be listed last, and + * those are the ones we want to search first. */ + for (guint i = ordered_table->len; i > 0; i--) { - GITypelib *typelib = (GITypelib*)value; + GITypelib *typelib = g_ptr_array_index (ordered_table, i - 1); + DirEntry *ret; + if (check_prefix) { - if (!gi_typelib_matches_gtype_name_prefix (typelib, data->gtype_name)) + if (!gi_typelib_matches_gtype_name_prefix (typelib, gtype_name)) continue; } - ret = gi_typelib_get_dir_entry_by_gtype_name (typelib, data->gtype_name); + ret = gi_typelib_get_dir_entry_by_gtype_name (typelib, gtype_name); if (ret) { - data->result_typelib = typelib; + *out_result_typelib = typelib; return ret; } } @@ -985,7 +982,8 @@ GIBaseInfo * gi_repository_find_by_gtype (GIRepository *repository, GType gtype) { - FindByGTypeData data; + const char *gtype_name; + GITypelib *result_typelib = NULL; GIBaseInfo *cached; DirEntry *entry; @@ -1001,8 +999,7 @@ gi_repository_find_by_gtype (GIRepository *repository, if (g_hash_table_contains (repository->unknown_gtypes, (gpointer)gtype)) return NULL; - data.gtype_name = g_type_name (gtype); - data.result_typelib = NULL; + gtype_name = g_type_name (gtype); /* Inside each typelib, we include the "C prefix" which acts as * a namespace mechanism. For GtkTreeView, the C prefix is Gtk. @@ -1011,9 +1008,9 @@ gi_repository_find_by_gtype (GIRepository *repository, * target type does not have this typelib's C prefix. Use this * assumption as our first attempt at locating the DirEntry. */ - entry = find_by_gtype (repository->typelibs, &data, TRUE); + entry = find_by_gtype (repository->ordered_typelibs, gtype_name, TRUE, &result_typelib); if (entry == NULL) - entry = find_by_gtype (repository->lazy_typelibs, &data, TRUE); + entry = find_by_gtype (repository->ordered_lazy_typelibs, gtype_name, TRUE, &result_typelib); /* Not every class library necessarily specifies a correct c_prefix, * so take a second pass. This time we will try a global lookup, @@ -1021,15 +1018,15 @@ gi_repository_find_by_gtype (GIRepository *repository, * See http://bugzilla.gnome.org/show_bug.cgi?id=564016 */ if (entry == NULL) - entry = find_by_gtype (repository->typelibs, &data, FALSE); + entry = find_by_gtype (repository->ordered_typelibs, gtype_name, FALSE, &result_typelib); if (entry == NULL) - entry = find_by_gtype (repository->lazy_typelibs, &data, FALSE); + entry = find_by_gtype (repository->ordered_lazy_typelibs, gtype_name, FALSE, &result_typelib); if (entry != NULL) { cached = gi_info_new_full (gi_typelib_blob_type_to_info_type (entry->blob_type), repository, - NULL, data.result_typelib, entry->offset); + NULL, result_typelib, entry->offset); g_hash_table_insert (repository->info_by_gtype, (gpointer) gtype, diff --git a/girepository/introspection/meson.build b/girepository/introspection/meson.build index b23d04884..f3ee28f09 100644 --- a/girepository/introspection/meson.build +++ b/girepository/introspection/meson.build @@ -261,6 +261,7 @@ if host_system == 'windows' '--identifier-prefix=GWin32' ], ) + gio_platform_gir = gio_win32_gir else gio_unix_gir_c_includes = [] foreach h: gio_unix_include_headers @@ -289,6 +290,7 @@ else '--identifier-prefix=GUnix' ], ) + gio_platform_gir = gio_unix_gir endif # GIRepository diff --git a/girepository/tests/meson.build b/girepository/tests/meson.build index a8e5b3d19..6fa947cfd 100644 --- a/girepository/tests/meson.build +++ b/girepository/tests/meson.build @@ -29,6 +29,11 @@ if enable_gir gio_gir, ] + gio_platform_gir_testing_dep = [ + gio_gir_testing_dep, + gio_platform_gir, + ] + girepository_gir_testing_dep = [ gio_gir_testing_dep, girepository_gir, @@ -46,7 +51,8 @@ if enable_gir 'depends': gobject_gir_testing_dep, }, 'repository' : { - 'depends': gio_gir_testing_dep, + 'depends': [gio_gir_testing_dep, gio_platform_gir_testing_dep], + 'dependencies': [libgio_dep], }, 'repository-search-paths' : { 'c_args': '-DGOBJECT_INTROSPECTION_LIBDIR="@0@"'.format(glib_libdir), diff --git a/girepository/tests/repository.c b/girepository/tests/repository.c index 565023147..af06088eb 100644 --- a/girepository/tests/repository.c +++ b/girepository/tests/repository.c @@ -35,6 +35,12 @@ #include "glib.h" #include "test-common.h" +#if defined(G_OS_UNIX) +#include "gio/gdesktopappinfo.h" +#elif defined(G_OS_WIN32) +#include "gio/gwin32inputstream.h" +#endif + static void test_repository_basic (RepositoryFixture *fx, const void *unused) @@ -777,6 +783,67 @@ test_repository_vfunc_info_with_invoker_on_object (RepositoryFixture *fx, g_clear_pointer (&invoker_info, gi_base_info_unref); } +static void +test_repository_find_by_gtype (RepositoryFixture *fx, + const void *unused) +{ + GIObjectInfo *object_info = NULL; + + g_test_summary ("Test finding a GType"); + + object_info = (GIObjectInfo *) gi_repository_find_by_gtype (fx->repository, G_TYPE_OBJECT); + g_assert_nonnull (object_info); + g_assert_true (GI_IS_OBJECT_INFO (object_info)); + g_assert_cmpstr (gi_base_info_get_name (GI_BASE_INFO (object_info)), ==, "Object"); + + g_clear_pointer (&object_info, gi_base_info_unref); + + /* Find it again; this time it should hit the cache. */ + object_info = (GIObjectInfo *) gi_repository_find_by_gtype (fx->repository, G_TYPE_OBJECT); + g_assert_nonnull (object_info); + g_assert_true (GI_IS_OBJECT_INFO (object_info)); + g_assert_cmpstr (gi_base_info_get_name (GI_BASE_INFO (object_info)), ==, "Object"); + + g_clear_pointer (&object_info, gi_base_info_unref); + + /* Try and find an unknown GType. */ + g_assert_null (gi_repository_find_by_gtype (fx->repository, GI_TYPE_BASE_INFO)); + + /* And check caching for unknown GTypes. */ + g_assert_null (gi_repository_find_by_gtype (fx->repository, GI_TYPE_BASE_INFO)); + + /* Now try and find one which will resolve in both Gio and GioUnix/GioWin32. + * The longest-named typelib should be returned. */ + { + GType platform_specific_type; + const char *expected_name, *expected_namespace; + +#if defined(G_OS_UNIX) + platform_specific_type = G_TYPE_DESKTOP_APP_INFO; + expected_name = "DesktopAppInfo"; + expected_namespace = "GioUnix"; +#elif defined(G_OS_WIN32) + platform_specific_type = G_TYPE_WIN32_INPUT_STREAM; + expected_name = "InputStream"; + expected_namespace = "GioWin32"; +#else + platform_specific_type = G_TYPE_INVALID; + expected_name = NULL; + expected_namespace = NULL; +#endif + + if (expected_name != NULL) + { + object_info = (GIObjectInfo *) gi_repository_find_by_gtype (fx->repository, platform_specific_type); + g_assert_nonnull (object_info); + g_assert_true (GI_IS_OBJECT_INFO (object_info)); + g_assert_cmpstr (gi_base_info_get_name (GI_BASE_INFO (object_info)), ==, expected_name); + g_assert_cmpstr (gi_base_info_get_namespace (GI_BASE_INFO (object_info)), ==, expected_namespace); + g_clear_pointer (&object_info, gi_base_info_unref); + } + } +} + int main (int argc, char *argv[]) @@ -804,6 +871,7 @@ main (int argc, ADD_REPOSITORY_TEST ("/repository/vfunc-info-with-no-invoker", test_repository_vfunc_info_with_no_invoker, &typelib_load_spec_gobject); ADD_REPOSITORY_TEST ("/repository/vfunc-info-with-invoker-on-interface", test_repository_vfunc_info_with_invoker_on_interface, &typelib_load_spec_gio); ADD_REPOSITORY_TEST ("/repository/vfunc-info-with-invoker-on-object", test_repository_vfunc_info_with_invoker_on_object, &typelib_load_spec_gio); + ADD_REPOSITORY_TEST ("/repository/find-by-gtype", test_repository_find_by_gtype, &typelib_load_spec_gio_platform); return g_test_run (); } diff --git a/girepository/tests/test-common.h b/girepository/tests/test-common.h index 9d31998d1..e7340c64b 100644 --- a/girepository/tests/test-common.h +++ b/girepository/tests/test-common.h @@ -36,6 +36,11 @@ typedef struct static const TypelibLoadSpec typelib_load_spec_glib = { "GLib", "2.0" }; static const TypelibLoadSpec typelib_load_spec_gobject = { "GObject", "2.0" }; static const TypelibLoadSpec typelib_load_spec_gio = { "Gio", "2.0" }; +#if defined(G_OS_UNIX) +static const TypelibLoadSpec typelib_load_spec_gio_platform = { "GioUnix", "2.0" }; +#elif defined(G_OS_WIN32) +static const TypelibLoadSpec typelib_load_spec_gio_platform = { "GioWin32", "2.0" }; +#endif void repository_init (int *argc, char **argv[]); From 400fac7c4acae3a10abbb1c2dd1de77675757542 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 2 May 2024 16:14:13 +0100 Subject: [PATCH 5/6] girepository: Make gi_repository_find_by_error_domain() deterministic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As with the previous commit, finding a `GIBaseInfo` matching the given error domain was non-deterministic because it iterated through a hash table of typelibs. Hash table iteration is non-deterministic. Change the method to instead use the `ordered_typelibs` and `ordered_lazy_typelibs` arrays to give deterministic iteration order. Add a unit test, although it can’t test the interesting case of an error domain which is present in both `GioUnix`/`GioWin32` and `Gio`, because no such error domain exists. Signed-off-by: Philip Withnall Helps: #3303 --- girepository/girepository.c | 57 +++++++++++++++------------------ girepository/tests/repository.c | 21 +++++++++++- 2 files changed, 46 insertions(+), 32 deletions(-) diff --git a/girepository/girepository.c b/girepository/girepository.c index e05012b4e..e74622e60 100644 --- a/girepository/girepository.c +++ b/girepository/girepository.c @@ -1078,28 +1078,27 @@ gi_repository_find_by_name (GIRepository *repository, NULL, typelib, entry->offset); } -typedef struct { - GIRepository *repository; - GQuark domain; - - GITypelib *result_typelib; - DirEntry *result; -} FindByErrorDomainData; - -static void -find_by_error_domain_foreach (gpointer key, - gpointer value, - gpointer datap) +static DirEntry * +find_by_error_domain (GPtrArray *ordered_typelibs, + GQuark target_domain, + GITypelib **out_typelib) { - GITypelib *typelib = (GITypelib*)value; - FindByErrorDomainData *data = datap; + /* Search in reverse order as the longest namespaces will be listed last, and + * those are the ones we want to search first. */ + for (guint i = ordered_typelibs->len; i > 0; i--) + { + GITypelib *typelib = g_ptr_array_index (ordered_typelibs, i - 1); + DirEntry *entry; - if (data->result != NULL) - return; + entry = gi_typelib_get_dir_entry_by_error_domain (typelib, target_domain); + if (entry != NULL) + { + *out_typelib = typelib; + return entry; + } + } - data->result = gi_typelib_get_dir_entry_by_error_domain (typelib, data->domain); - if (data->result) - data->result_typelib = typelib; + return NULL; } /** @@ -1122,8 +1121,9 @@ GIEnumInfo * gi_repository_find_by_error_domain (GIRepository *repository, GQuark domain) { - FindByErrorDomainData data; GIEnumInfo *cached; + DirEntry *result = NULL; + GITypelib *result_typelib = NULL; g_return_val_if_fail (GI_IS_REPOSITORY (repository), NULL); @@ -1133,20 +1133,15 @@ gi_repository_find_by_error_domain (GIRepository *repository, if (cached != NULL) return (GIEnumInfo *) gi_base_info_ref ((GIBaseInfo *)cached); - data.repository = repository; - data.domain = domain; - data.result_typelib = NULL; - data.result = NULL; + result = find_by_error_domain (repository->ordered_typelibs, domain, &result_typelib); + if (result == NULL) + result = find_by_error_domain (repository->ordered_lazy_typelibs, domain, &result_typelib); - g_hash_table_foreach (repository->typelibs, find_by_error_domain_foreach, &data); - if (data.result == NULL) - g_hash_table_foreach (repository->lazy_typelibs, find_by_error_domain_foreach, &data); - - if (data.result != NULL) + if (result != NULL) { - cached = (GIEnumInfo *) gi_info_new_full (gi_typelib_blob_type_to_info_type (data.result->blob_type), + cached = (GIEnumInfo *) gi_info_new_full (gi_typelib_blob_type_to_info_type (result->blob_type), repository, - NULL, data.result_typelib, data.result->offset); + NULL, result_typelib, result->offset); g_hash_table_insert (repository->info_by_error_domain, GUINT_TO_POINTER (domain), diff --git a/girepository/tests/repository.c b/girepository/tests/repository.c index af06088eb..55cbcb0ce 100644 --- a/girepository/tests/repository.c +++ b/girepository/tests/repository.c @@ -513,12 +513,31 @@ test_repository_error_quark (RepositoryFixture *fx, g_test_summary ("Test finding an error quark by error domain"); + /* Find a simple error domain. */ error_info = gi_repository_find_by_error_domain (fx->repository, G_RESOLVER_ERROR); g_assert_nonnull (error_info); g_assert_true (GI_IS_ENUM_INFO (error_info)); g_assert_cmpstr (gi_base_info_get_name (GI_BASE_INFO (error_info)), ==, "ResolverError"); g_clear_pointer (&error_info, gi_base_info_unref); + + /* Find again to check the caching. */ + error_info = gi_repository_find_by_error_domain (fx->repository, G_RESOLVER_ERROR); + g_assert_nonnull (error_info); + g_assert_true (GI_IS_ENUM_INFO (error_info)); + g_assert_cmpstr (gi_base_info_get_name (GI_BASE_INFO (error_info)), ==, "ResolverError"); + + g_clear_pointer (&error_info, gi_base_info_unref); + + /* Try and find an unknown error domain. */ + g_assert_null (gi_repository_find_by_error_domain (fx->repository, GI_REPOSITORY_ERROR)); + + /* And check caching for unknown error domains. */ + g_assert_null (gi_repository_find_by_error_domain (fx->repository, GI_REPOSITORY_ERROR)); + + /* It would be good to try and find one which will resolve in both Gio and + * GioUnix/GioWin32, but neither of the platform-specific GIRs actually define + * any error domains at the moment. */ } static void @@ -861,7 +880,7 @@ main (int argc, ADD_REPOSITORY_TEST ("/repository/constructor-return-type", test_repository_constructor_return_type, &typelib_load_spec_gobject); ADD_REPOSITORY_TEST ("/repository/enum-info-c-identifier", test_repository_enum_info_c_identifier, &typelib_load_spec_glib); ADD_REPOSITORY_TEST ("/repository/enum-info-static-methods", test_repository_enum_info_static_methods, &typelib_load_spec_glib); - ADD_REPOSITORY_TEST ("/repository/error-quark", test_repository_error_quark, &typelib_load_spec_gio); + ADD_REPOSITORY_TEST ("/repository/error-quark", test_repository_error_quark, &typelib_load_spec_gio_platform); ADD_REPOSITORY_TEST ("/repository/flags-info-c-identifier", test_repository_flags_info_c_identifier, &typelib_load_spec_gobject); ADD_REPOSITORY_TEST ("/repository/fundamental-ref-func", test_repository_fundamental_ref_func, &typelib_load_spec_gobject); ADD_REPOSITORY_TEST ("/repository/instance-method-ownership-transfer", test_repository_instance_method_ownership_transfer, &typelib_load_spec_gio); From 3858b8b8155fc5ff6f15ae6cee5050623bd0761a Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 2 May 2024 16:30:09 +0100 Subject: [PATCH 6/6] girepository: Make gi_repository_get_loaded_namespaces() deterministic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As with the previous two commits, the results of calling `gi_repository_get_loaded_namespaces()` were previously non-deterministic due to being generated by iterating over a hash table, which has a non-deterministic iteration order. Fix that by using the new `ordered_typelibs` and `ordered_lazy_typelibs` arrays to provide deterministic ordering. At the same time, significantly reduce the number of allocations needed to build the return value — previously the results would be built as a linked list before being turned into an array. The linked list is now banished to history. Add some more unit tests to maximise test coverage of this method. Signed-off-by: Philip Withnall Fixes: #3303 --- girepository/girepository.c | 29 +++++++++++++++-------------- girepository/tests/repository.c | 30 ++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 14 deletions(-) diff --git a/girepository/girepository.c b/girepository/girepository.c index e74622e60..0cf6da148 100644 --- a/girepository/girepository.c +++ b/girepository/girepository.c @@ -1232,13 +1232,16 @@ gi_repository_get_object_gtype_interfaces (GIRepository *repository, } static void -collect_namespaces (gpointer key, - gpointer value, - gpointer data) +collect_namespaces (GPtrArray *ordered_typelibs, + char **names, + size_t *inout_i) { - GList **list = data; - - *list = g_list_append (*list, key); + for (guint j = 0; j < ordered_typelibs->len; j++) + { + GITypelib *typelib = g_ptr_array_index (ordered_typelibs, j); + const char *namespace = gi_typelib_get_namespace (typelib); + names[(*inout_i)++] = g_strdup (namespace); + } } /** @@ -1260,20 +1263,18 @@ char ** gi_repository_get_loaded_namespaces (GIRepository *repository, size_t *n_namespaces_out) { - GList *l, *list = NULL; char **names; size_t i; + size_t n_typelibs; g_return_val_if_fail (GI_IS_REPOSITORY (repository), NULL); - g_hash_table_foreach (repository->typelibs, collect_namespaces, &list); - g_hash_table_foreach (repository->lazy_typelibs, collect_namespaces, &list); - - names = g_malloc0 (sizeof (char *) * (g_list_length (list) + 1)); + n_typelibs = repository->ordered_typelibs->len + repository->ordered_lazy_typelibs->len; + names = g_malloc0 (sizeof (char *) * (n_typelibs + 1)); i = 0; - for (l = list; l; l = l->next) - names[i++] = g_strdup (l->data); - g_list_free (list); + + collect_namespaces (repository->ordered_typelibs, names, &i); + collect_namespaces (repository->ordered_lazy_typelibs, names, &i); if (n_namespaces_out != NULL) *n_namespaces_out = i; diff --git a/girepository/tests/repository.c b/girepository/tests/repository.c index 55cbcb0ce..c79b0df9a 100644 --- a/girepository/tests/repository.c +++ b/girepository/tests/repository.c @@ -863,6 +863,35 @@ test_repository_find_by_gtype (RepositoryFixture *fx, } } +static void +test_repository_loaded_namespaces (RepositoryFixture *fx, + const void *unused) +{ + char **namespaces; + size_t n_namespaces; + + /* These should be in alphabetical order */ +#if defined(G_OS_UNIX) + const char *expected_namespaces[] = { "GLib", "GModule", "GObject", "Gio", "GioUnix", NULL }; +#elif defined(G_OS_WIN32) + const char *expected_namespaces[] = { "GLib", "GModule", "GObject", "Gio", "GioWin32", NULL }; +#else + const char *expected_namespaces[] = { "GLib", "GModule", "GObject", "Gio", NULL }; +#endif + + g_test_summary ("Test listing loaded namespaces"); + + namespaces = gi_repository_get_loaded_namespaces (fx->repository, &n_namespaces); + g_assert_cmpstrv (namespaces, expected_namespaces); + g_assert_cmpuint (n_namespaces, ==, g_strv_length ((char **) expected_namespaces)); + g_strfreev (namespaces); + + /* Test again but without passing `n_namespaces`. */ + namespaces = gi_repository_get_loaded_namespaces (fx->repository, NULL); + g_assert_cmpstrv (namespaces, expected_namespaces); + g_strfreev (namespaces); +} + int main (int argc, char *argv[]) @@ -891,6 +920,7 @@ main (int argc, ADD_REPOSITORY_TEST ("/repository/vfunc-info-with-invoker-on-interface", test_repository_vfunc_info_with_invoker_on_interface, &typelib_load_spec_gio); ADD_REPOSITORY_TEST ("/repository/vfunc-info-with-invoker-on-object", test_repository_vfunc_info_with_invoker_on_object, &typelib_load_spec_gio); ADD_REPOSITORY_TEST ("/repository/find-by-gtype", test_repository_find_by_gtype, &typelib_load_spec_gio_platform); + ADD_REPOSITORY_TEST ("/repository/loaded-namespaces", test_repository_loaded_namespaces, &typelib_load_spec_gio_platform); return g_test_run (); }