From 0d199d619975a07282ba1264cb9373e776010a87 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 2 May 2024 16:14:13 +0100 Subject: [PATCH] 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);