From ee589aaa3206041bf0a5b7dbbc487208c1a48530 Mon Sep 17 00:00:00 2001 From: nitinosiris Date: Wed, 23 Jun 2021 23:33:31 +0530 Subject: [PATCH 1/2] API: Add g_module_open_full() g_module_open_full() is wrapper around g_module_open() function which returns a GError in case of failure. Closes #203 --- docs/reference/glib/glib-sections.txt | 5 ++ gmodule/gmodule-ar.c | 10 ++- gmodule/gmodule-dl.c | 11 ++- gmodule/gmodule-win32.c | 29 ++++--- gmodule/gmodule.c | 117 +++++++++++++++----------- gmodule/gmodule.h | 25 ++++++ 6 files changed, 132 insertions(+), 65 deletions(-) diff --git a/docs/reference/glib/glib-sections.txt b/docs/reference/glib/glib-sections.txt index 406041253..9c45e5e4b 100644 --- a/docs/reference/glib/glib-sections.txt +++ b/docs/reference/glib/glib-sections.txt @@ -2490,9 +2490,12 @@ g_bookmark_file_error_quark modules gmodule.h GModule +GModuleError +G_MODULE_ERROR g_module_supported g_module_build_path g_module_open +g_module_open_full GModuleFlags g_module_symbol g_module_name @@ -2505,6 +2508,8 @@ GModuleUnload G_MODULE_SUFFIX G_MODULE_EXPORT G_MODULE_IMPORT + +g_module_error_quark
diff --git a/gmodule/gmodule-ar.c b/gmodule/gmodule-ar.c index 381eef4db..8c3f15042 100644 --- a/gmodule/gmodule-ar.c +++ b/gmodule/gmodule-ar.c @@ -100,7 +100,8 @@ exit: static gpointer _g_module_open (const gchar *file_name, gboolean bind_lazy, - gboolean bind_local) + gboolean bind_local, + GError **error) { gpointer handle; gchar* member; @@ -123,7 +124,12 @@ _g_module_open (const gchar *file_name, g_free (full_name); if (!handle) - g_module_set_error (fetch_dlerror (TRUE)); + { + const gchar *message = fetch_dlerror (TRUE); + + g_module_set_error (message); + g_set_error_literal (error, G_MODULE_ERROR, G_MODULE_ERROR_FAILED, message); + } return handle; } diff --git a/gmodule/gmodule-dl.c b/gmodule/gmodule-dl.c index c1408212a..26f6ba029 100644 --- a/gmodule/gmodule-dl.c +++ b/gmodule/gmodule-dl.c @@ -127,7 +127,8 @@ fetch_dlerror (gboolean replace_null) static gpointer _g_module_open (const gchar *file_name, gboolean bind_lazy, - gboolean bind_local) + gboolean bind_local, + GError **error) { gpointer handle; @@ -135,7 +136,13 @@ _g_module_open (const gchar *file_name, handle = dlopen (file_name, (bind_local ? 0 : RTLD_GLOBAL) | (bind_lazy ? RTLD_LAZY : RTLD_NOW)); if (!handle) - g_module_set_error (fetch_dlerror (TRUE)); + { + const gchar *message = fetch_dlerror (TRUE); + + g_module_set_error (message); + g_set_error_literal (error, G_MODULE_ERROR, G_MODULE_ERROR_FAILED, message); + } + unlock_dlerror (); return handle; diff --git a/gmodule/gmodule-win32.c b/gmodule/gmodule-win32.c index 2aea8bae9..cb5194961 100644 --- a/gmodule/gmodule-win32.c +++ b/gmodule/gmodule-win32.c @@ -30,6 +30,7 @@ */ #include "config.h" +#include #include #include @@ -39,34 +40,38 @@ #include #endif -static void G_GNUC_PRINTF (1, 2) -set_error (const gchar *format, - ...) +static void G_GNUC_PRINTF (2, 3) +set_error (GError **error, + const gchar *format, + ...) { - gchar *error; + gchar *win32_error; gchar *detail; gchar *message; va_list args; - error = g_win32_error_message (GetLastError ()); + win32_error = g_win32_error_message (GetLastError ()); va_start (args, format); detail = g_strdup_vprintf (format, args); va_end (args); - message = g_strconcat (detail, error, NULL); + message = g_strconcat (detail, win32_error, NULL); g_module_set_error (message); + g_set_error_literal (error, G_MODULE_ERROR, G_MODULE_ERROR_FAILED, message); + g_free (message); g_free (detail); - g_free (error); + g_free (win32_error); } /* --- functions --- */ static gpointer _g_module_open (const gchar *file_name, gboolean bind_lazy, - gboolean bind_local) + gboolean bind_local, + GError **error) { HINSTANCE handle; wchar_t *wfilename; @@ -83,7 +88,7 @@ _g_module_open (const gchar *file_name, /* suppress error dialog */ success = SetThreadErrorMode (SEM_NOOPENFILEERRORBOX | SEM_FAILCRITICALERRORS, &old_mode); if (!success) - set_error (""); + set_error (error, ""); /* When building for UWP, load app asset DLLs instead of filesystem DLLs. * Needs MSVC, Windows 8 and newer, and is only usable from apps. */ @@ -98,7 +103,7 @@ _g_module_open (const gchar *file_name, g_free (wfilename); if (!handle) - set_error ("'%s': ", file_name); + set_error (error, "'%s': ", file_name); return handle; } @@ -117,7 +122,7 @@ _g_module_close (gpointer handle) { if (handle != null_module_handle) if (!FreeLibrary (handle)) - set_error (""); + set_error (NULL, ""); } static gpointer @@ -189,7 +194,7 @@ _g_module_symbol (gpointer handle, p = GetProcAddress (handle, symbol_name); if (!p) - set_error (""); + set_error (NULL, ""); return p; } diff --git a/gmodule/gmodule.c b/gmodule/gmodule.c index 3c67e2e54..4a4e279a1 100644 --- a/gmodule/gmodule.c +++ b/gmodule/gmodule.c @@ -204,9 +204,10 @@ struct _GModule /* --- prototypes --- */ -static gpointer _g_module_open (const gchar *file_name, - gboolean bind_lazy, - gboolean bind_local); +static gpointer _g_module_open (const gchar *file_name, + gboolean bind_lazy, + gboolean bind_local, + GError **error); static void _g_module_close (gpointer handle); static gpointer _g_module_self (void); static gpointer _g_module_symbol (gpointer handle, @@ -289,10 +290,12 @@ g_module_set_error (const gchar *error) #define SUPPORT_OR_RETURN(rv) { g_module_set_error ("dynamic modules are " \ "not supported by this system"); return rv; } static gpointer -_g_module_open (const gchar *file_name, - gboolean bind_lazy, - gboolean bind_local) +_g_module_open (const gchar *file_name, + gboolean bind_lazy, + gboolean bind_local, + GError **error) { + g_module_set_error (NULL); return NULL; } static void @@ -318,6 +321,15 @@ _g_module_build_path (const gchar *directory, } #endif /* no implementation */ +/** + * G_MODULE_ERROR: + * + * The error domain of the #GModule API. + * + * Since: 2.70 + */ +G_DEFINE_QUARK (g-module-error-quark, g_module_error) + /* --- functions --- */ /** @@ -425,17 +437,6 @@ parse_libtool_archive (const gchar* libtool_name) return name; } -static inline gboolean -str_check_suffix (const gchar* string, - const gchar* suffix) -{ - gsize string_len = strlen (string); - gsize suffix_len = strlen (suffix); - - return string_len >= suffix_len && - strcmp (string + string_len - suffix_len, suffix) == 0; -} - enum { G_MODULE_DEBUG_RESIDENT_MODULES = 1 << 0, @@ -462,36 +463,42 @@ _g_module_debug_init (void) static GRecMutex g_module_global_lock; /** - * g_module_open: + * g_module_open_full: * @file_name: (nullable): the name of the file containing the module, or %NULL * to obtain a #GModule representing the main program itself * @flags: the flags used for opening the module. This can be the * logical OR of any of the #GModuleFlags + * @error: #GError. * * Opens a module. If the module has already been opened, * its reference count is incremented. * - * First of all g_module_open() tries to open @file_name as a module. + * First of all g_module_open_full() tries to open @file_name as a module. * If that fails and @file_name has the ".la"-suffix (and is a libtool * archive) it tries to open the corresponding module. If that fails * and it doesn't have the proper module suffix for the platform * (#G_MODULE_SUFFIX), this suffix will be appended and the corresponding * module will be opened. If that fails and @file_name doesn't have the - * ".la"-suffix, this suffix is appended and g_module_open() tries to open + * ".la"-suffix, this suffix is appended and g_module_open_full() tries to open * the corresponding module. If eventually that fails as well, %NULL is * returned. * * Returns: a #GModule on success, or %NULL on failure + * + * Since: 2.70 */ GModule* -g_module_open (const gchar *file_name, - GModuleFlags flags) +g_module_open_full (const gchar *file_name, + GModuleFlags flags, + GError **error) { GModule *module; gpointer handle = NULL; gchar *name = NULL; SUPPORT_OR_RETURN (NULL); + + g_return_val_if_fail (error == NULL || *error == NULL, NULL); g_rec_mutex_lock (&g_module_global_lock); @@ -577,30 +584,23 @@ g_module_open (const gchar *file_name, } /* ok, try loading the module */ - if (name) - { - /* if it's a libtool archive, figure library file to load */ - if (str_check_suffix (name, ".la")) /* libtool archive? */ - { - gchar *real_name = parse_libtool_archive (name); + g_assert (name != NULL); - /* real_name might be NULL, but then module error is already set */ - if (real_name) - { - g_free (name); - name = real_name; - } - } - if (name) - handle = _g_module_open (name, (flags & G_MODULE_BIND_LAZY) != 0, - (flags & G_MODULE_BIND_LOCAL) != 0); - } - else + /* if it's a libtool archive, figure library file to load */ + if (g_str_has_suffix (name, ".la")) /* libtool archive? */ { - gchar *display_file_name = g_filename_display_name (file_name); - g_module_set_error_unduped (g_strdup_printf ("unable to access file \"%s\"", display_file_name)); - g_free (display_file_name); + gchar *real_name = parse_libtool_archive (name); + + /* real_name might be NULL, but then module error is already set */ + if (real_name) + { + g_free (name); + name = real_name; + } } + + handle = _g_module_open (name, (flags & G_MODULE_BIND_LAZY) != 0, + (flags & G_MODULE_BIND_LOCAL) != 0, error); g_free (name); if (handle) @@ -643,15 +643,16 @@ g_module_open (const gchar *file_name, if (check_failed) { - gchar *error; + gchar *temp_error; - error = g_strconcat ("GModule (", file_name, ") ", - "initialization check failed: ", - check_failed, NULL); + temp_error = g_strconcat ("GModule (", file_name, ") ", + "initialization check failed: ", + check_failed, NULL); g_module_close (module); module = NULL; - g_module_set_error (error); - g_free (error); + g_module_set_error (temp_error); + g_set_error_literal (error, G_MODULE_ERROR, G_MODULE_ERROR_CHECK_FAILED, temp_error); + g_free (temp_error); } else g_module_set_error (saved_error); @@ -667,6 +668,24 @@ g_module_open (const gchar *file_name, return module; } +/** + * g_module_open: + * @file_name: (nullable): the name of the file containing the module, or %NULL + * to obtain a #GModule representing the main program itself + * @flags: the flags used for opening the module. This can be the + * logical OR of any of the #GModuleFlags. + * + * A thin wrapper function around g_module_open_full() + * + * Returns: a #GModule on success, or %NULL on failure + */ +GModule * +g_module_open (const gchar *file_name, + GModuleFlags flags) +{ + return g_module_open_full (file_name, flags, NULL); +} + /** * g_module_close: * @module: a #GModule to close diff --git a/gmodule/gmodule.h b/gmodule/gmodule.h index 486684deb..c5fd03334 100644 --- a/gmodule/gmodule.h +++ b/gmodule/gmodule.h @@ -66,6 +66,26 @@ typedef struct _GModule GModule; typedef const gchar* (*GModuleCheckInit) (GModule *module); typedef void (*GModuleUnload) (GModule *module); +#define G_MODULE_ERROR g_module_error_quark () GLIB_AVAILABLE_MACRO_IN_2_70 +GLIB_AVAILABLE_IN_2_70 +GQuark g_module_error_quark (void); + +/** + * GModuleError: + * @G_MODULE_ERROR_FAILED: there was an error loading or opening a module file + * @G_MODULE_ERROR_CHECK_FAILED: a module returned an error from its `g_module_check_init()` function + * + * Errors returned by g_module_open_full(). + * + * Since: 2.70 + */ +typedef enum +{ + G_MODULE_ERROR_FAILED, + G_MODULE_ERROR_CHECK_FAILED, +} GModuleError +GLIB_AVAILABLE_ENUMERATOR_IN_2_70; + /* return TRUE if dynamic module loading is supported */ GLIB_AVAILABLE_IN_ALL gboolean g_module_supported (void) G_GNUC_CONST; @@ -75,6 +95,11 @@ GLIB_AVAILABLE_IN_ALL GModule* g_module_open (const gchar *file_name, GModuleFlags flags); +GLIB_AVAILABLE_IN_2_70 +GModule *g_module_open_full (const gchar *file_name, + GModuleFlags flags, + GError **error); + /* close a previously opened module, returns TRUE on success */ GLIB_AVAILABLE_IN_ALL gboolean g_module_close (GModule *module); From be012a80676e5b15f6e02c2553073901d138a6bd Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 21 Jul 2021 21:54:35 +0100 Subject: [PATCH 2/2] giomodule: Port to new g_module_open_full() API Port all existing calls in GLib to the new API so that they can receive more detailed error information (although none of them actually make use of it at the moment). This also serves to test the new API better through use. Signed-off-by: Philip Withnall Helps: #203 --- gio/gio-querymodules.c | 8 +++++++- gio/giomodule.c | 6 ++++-- tests/module-test.c | 10 +++++++--- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/gio/gio-querymodules.c b/gio/gio-querymodules.c index eb5094f4d..cbeb9758e 100644 --- a/gio/gio-querymodules.c +++ b/gio/gio-querymodules.c @@ -81,7 +81,7 @@ query_dir (const char *dirname) continue; path = g_build_filename (dirname, name, NULL); - module = g_module_open (path, G_MODULE_BIND_LAZY | G_MODULE_BIND_LOCAL); + module = g_module_open_full (path, G_MODULE_BIND_LAZY | G_MODULE_BIND_LOCAL, &error); g_free (path); if (module) @@ -119,6 +119,12 @@ query_dir (const char *dirname) g_module_close (module); } + else + { + g_debug ("Failed to open module %s: %s", name, error->message); + } + + g_clear_error (&error); } g_dir_close (dir); diff --git a/gio/giomodule.c b/gio/giomodule.c index d3b5e482a..c1d451b5c 100644 --- a/gio/giomodule.c +++ b/gio/giomodule.c @@ -342,6 +342,7 @@ static gboolean g_io_module_load_module (GTypeModule *gmodule) { GIOModule *module = G_IO_MODULE (gmodule); + GError *error = NULL; if (!module->filename) { @@ -349,11 +350,12 @@ g_io_module_load_module (GTypeModule *gmodule) return FALSE; } - module->library = g_module_open (module->filename, G_MODULE_BIND_LAZY | G_MODULE_BIND_LOCAL); + module->library = g_module_open_full (module->filename, G_MODULE_BIND_LAZY | G_MODULE_BIND_LOCAL, &error); if (!module->library) { - g_printerr ("%s\n", g_module_error ()); + g_printerr ("%s\n", error->message); + g_clear_error (&error); return FALSE; } diff --git a/tests/module-test.c b/tests/module-test.c index e0e6423f6..62473d29d 100644 --- a/tests/module-test.c +++ b/tests/module-test.c @@ -84,6 +84,7 @@ main (int argc, gchar *plugin_a, *plugin_b; SimpleFunc f_a, f_b, f_self; GModuleFunc gmod_f; + GError *error = NULL; g_test_init (&argc, &argv, NULL); @@ -95,18 +96,21 @@ main (int argc, /* module handles */ - module_self = g_module_open (NULL, G_MODULE_BIND_LAZY); + module_self = g_module_open_full (NULL, G_MODULE_BIND_LAZY, &error); + g_assert_no_error (error); if (!module_self) g_error ("error: %s", g_module_error ()); if (!g_module_symbol (module_self, "g_module_close", (gpointer *) &f_self)) g_error ("error: %s", g_module_error ()); - module_a = g_module_open (plugin_a, G_MODULE_BIND_LAZY); + module_a = g_module_open_full (plugin_a, G_MODULE_BIND_LAZY, &error); + g_assert_no_error (error); if (!module_a) g_error ("error: %s", g_module_error ()); - module_b = g_module_open (plugin_b, G_MODULE_BIND_LAZY); + module_b = g_module_open_full (plugin_b, G_MODULE_BIND_LAZY, &error); + g_assert_no_error (error); if (!module_b) g_error ("error: %s", g_module_error ());