From 81b16a88d1d22c735315e6d31802dd0c37277840 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 2 May 2024 16:03:24 +0100 Subject: [PATCH] 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 0a217e75f..9e2d0f2bb 100644 --- a/girepository/introspection/meson.build +++ b/girepository/introspection/meson.build @@ -268,6 +268,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 @@ -297,6 +298,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[]);