Branch: refs/heads/master Author: Thomas Martitz kugel@rockbox.org Committer: Thomas Martitz kugel@rockbox.org Date: Mon, 05 Oct 2015 20:11:12 UTC Commit: bdaab9c8378913016d8bc6c76d2100e6d6a9baab https://github.com/geany/geany/commit/bdaab9c8378913016d8bc6c76d2100e6d6a9ba...
Log Message: ----------- plugins: generic load_data instead of module pointer in Plugin struct
Being a GModule is actually a detail of standard plugins. Future proxy plugins might need different handles. Therefore replace the module field with a more generic pointer and encapsulate the GModule detail further.
This pointer shall be returned from GeanyProxyFuncs::load and is passed back to GeanyProxyFuncs::unload, and isn't interpreted by Geany.
Modified Paths: -------------- src/plugindata.h src/pluginprivate.h src/plugins.c src/pluginutils.c
Modified: src/plugindata.h 4 lines changed, 2 insertions(+), 2 deletions(-) =================================================================== @@ -350,8 +350,8 @@ void geany_plugin_set_data(GeanyPlugin *plugin, gpointer data, GDestroyNotify fr /* Hooks that need to be implemented for every proxy */ typedef struct _GeanyProxyFuncs { - void (*load) (GeanyPlugin *proxy, GeanyPlugin *subplugins, const gchar *filename, gpointer pdata); - void (*unload) (GeanyPlugin *proxy, GeanyPlugin *subplugins, gpointer pdata); + gpointer (*load) (GeanyPlugin *proxy, GeanyPlugin *subplugin, const gchar *filename, gpointer pdata); + void (*unload) (GeanyPlugin *proxy, GeanyPlugin *subplugin, gpointer load_data, gpointer pdata); } GeanyProxyFuncs;
Modified: src/pluginprivate.h 5 lines changed, 4 insertions(+), 1 deletions(-) =================================================================== @@ -50,7 +50,6 @@ typedef struct GeanyPluginPrivate Plugin; /* shorter alias */
typedef struct GeanyPluginPrivate { - GModule *module; gchar *filename; /* plugin filename (/path/libname.so) */ PluginInfo info; /* plugin name, description, etc */ GeanyPlugin public; /* fields the plugin can read */ @@ -72,6 +71,8 @@ typedef struct GeanyPluginPrivate /* proxy plugin support */ GeanyProxyFuncs proxy_cbs; Plugin *proxy; /* The proxy that handles this plugin */ + gpointer proxy_data; /* Data passed to the proxy hooks of above proxy, so + * this gives the proxy a pointer to each plugin */ } GeanyPluginPrivate;
@@ -80,6 +81,8 @@ GeanyPluginPrivate; #define PLUGIN_HAS_LOAD_DATA(p) (((p)->flags & LOAD_DATA) != 0)
void plugin_watch_object(Plugin *plugin, gpointer object); +void plugin_make_resident(Plugin *plugin); +gpointer plugin_get_module_symbol(Plugin *plugin, const gchar *sym);
G_END_DECLS
Modified: src/plugins.c 100 lines changed, 78 insertions(+), 22 deletions(-) =================================================================== @@ -81,8 +81,8 @@ typedef struct { } PluginProxy;
-static void plugin_load_gmodule(GeanyPlugin *proxy, GeanyPlugin *plugin, const gchar *filename, gpointer pdata); -static void plugin_unload_gmodule(GeanyPlugin *proxy, GeanyPlugin *plugin, gpointer pdata); +static gpointer plugin_load_gmodule(GeanyPlugin *proxy, GeanyPlugin *plugin, const gchar *filename, gpointer pdata); +static void plugin_unload_gmodule(GeanyPlugin *proxy, GeanyPlugin *plugin, gpointer load_data, gpointer pdata);
static Plugin builtin_so_proxy_plugin = { .proxy_cbs = { @@ -190,22 +190,27 @@ static Plugin *find_active_plugin_by_name(const gchar *filename) static gboolean plugin_check_version(Plugin *plugin, int plugin_version_code) { - const gchar *name = g_module_name(plugin->module); + gboolean ret = TRUE; if (plugin_version_code < 0) { + gchar *name = g_path_get_basename(plugin->filename); msgwin_status_add(_("The plugin "%s" is not binary compatible with this " "release of Geany - please recompile it."), name); geany_debug("Plugin "%s" is not binary compatible with this " "release of Geany - recompile it.", name); - return FALSE; + ret = FALSE; + g_free(name); } else if (plugin_version_code > GEANY_API_VERSION) { + gchar *name = g_path_get_basename(plugin->filename); geany_debug("Plugin "%s" requires a newer version of Geany (API >= v%d).", name, plugin_version_code); - return FALSE; + ret = FALSE; + g_free(name); } - return TRUE; + + return ret; }
@@ -239,9 +244,10 @@ static void read_key_group(Plugin *plugin) { GeanyKeyGroupInfo *p_key_info; GeanyKeyGroup **p_key_group; + GModule *module = plugin->proxy_data;
- g_module_symbol(plugin->module, "plugin_key_group_info", (void *) &p_key_info); - g_module_symbol(plugin->module, "plugin_key_group", (void *) &p_key_group); + g_module_symbol(module, "plugin_key_group_info", (void *) &p_key_info); + g_module_symbol(module, "plugin_key_group", (void *) &p_key_group); if (p_key_info && p_key_group) { GeanyKeyGroupInfo *key_info = p_key_info; @@ -329,8 +335,10 @@ gboolean geany_plugin_register(GeanyPlugin *plugin, gint api_version, gint min_a /* Only init and cleanup callbacks are truly mandatory. */ if (! cbs->init || ! cbs->cleanup) { - geany_debug("Plugin '%s' has no %s function - ignoring plugin!", - g_module_name(p->module), cbs->init ? "cleanup" : "init"); + gchar *name = g_path_get_basename(p->filename); + geany_debug("Plugin '%s' has no %s function - ignoring plugin!", name, + cbs->init ? "cleanup" : "init"); + g_free(name); } else { @@ -510,15 +518,16 @@ plugin_load(Plugin *plugin) GeanyPlugin **p_geany_plugin; PluginInfo **p_info; PluginFields **plugin_fields; + GModule *module = plugin->proxy_data; /* set these symbols before plugin_init() is called * we don't set geany_data since it is set directly by plugin_new() */ - g_module_symbol(plugin->module, "geany_plugin", (void *) &p_geany_plugin); + g_module_symbol(module, "geany_plugin", (void *) &p_geany_plugin); if (p_geany_plugin) *p_geany_plugin = &plugin->public; - g_module_symbol(plugin->module, "plugin_info", (void *) &p_info); + g_module_symbol(module, "plugin_info", (void *) &p_info); if (p_info) *p_info = &plugin->info; - g_module_symbol(plugin->module, "plugin_fields", (void *) &plugin_fields); + g_module_symbol(module, "plugin_fields", (void *) &plugin_fields); if (plugin_fields) *plugin_fields = &plugin->fields; read_key_group(plugin); @@ -554,7 +563,7 @@ plugin_load(Plugin *plugin) }
-static void plugin_load_gmodule(GeanyPlugin *proxy, GeanyPlugin *subplugin, const gchar *fname, gpointer pdata) +static gpointer plugin_load_gmodule(GeanyPlugin *proxy, GeanyPlugin *subplugin, const gchar *fname, gpointer pdata) { GModule *module; void (*p_geany_load_module)(GeanyPlugin *); @@ -570,10 +579,9 @@ static void plugin_load_gmodule(GeanyPlugin *proxy, GeanyPlugin *subplugin, cons if (!module) { geany_debug("Can't load plugin: %s", g_module_error()); - return; + return NULL; }
- subplugin->priv->module = module; /*geany_debug("Initializing plugin '%s'", plugin->info.name);*/ g_module_symbol(module, "geany_load_module", (void *) &p_geany_load_module); if (p_geany_load_module) @@ -592,14 +600,15 @@ static void plugin_load_gmodule(GeanyPlugin *proxy, GeanyPlugin *subplugin, cons register_legacy_plugin(subplugin->priv, module); } /* We actually check the LOADED_OK flag later */ + return module; }
-static void plugin_unload_gmodule(GeanyPlugin *proxy, GeanyPlugin *subplugin, gpointer pdata) +static void plugin_unload_gmodule(GeanyPlugin *proxy, GeanyPlugin *subplugin, gpointer load_data, gpointer pdata) { - GModule *module = subplugin->priv->module; + GModule *module = (GModule *) load_data;
- g_return_if_fail(module); + g_return_if_fail(module != NULL);
if (! g_module_close(module)) g_warning("%s: %s", subplugin->priv->filename, g_module_error()); @@ -653,7 +662,7 @@ plugin_new(Plugin *proxy, const gchar *fname, gboolean load_plugin, gboolean add
/* Load plugin, this should read its name etc. It must also call * geany_plugin_register() for the following PLUGIN_LOADED_OK condition */ - proxy->proxy_cbs.load(&proxy->public, &plugin->public, fname, proxy->cb_data); + plugin->proxy_data = proxy->proxy_cbs.load(&proxy->public, &plugin->public, fname, proxy->cb_data);
if (! PLUGIN_LOADED_OK(plugin)) { @@ -669,6 +678,16 @@ plugin_new(Plugin *proxy, const gchar *fname, gboolean load_plugin, gboolean add goto err_unload; }
+ /* cb_data_destroy() frees plugin->cb_data. If that pointer also passed to unload() afterwards + * then that would become a use-after-free. Disallow this combination. If a proxy + * needs the same pointer it must not use a destroy func but free manually in its unload(). */ + if (plugin->proxy_data == proxy->cb_data && plugin->cb_data_destroy) + { + geany_debug("Proxy of plugin "%s" specified invalid data - ignoring plugin!", fname); + plugin->proxy_data = NULL; + goto err_unload; + } + if (load_plugin && !plugin_load(plugin)) { /* Handle failing init same as failing to load for now. In future we @@ -685,7 +704,7 @@ plugin_new(Plugin *proxy, const gchar *fname, gboolean load_plugin, gboolean add err_unload: if (plugin->cb_data_destroy) plugin->cb_data_destroy(plugin->cb_data); - proxy->proxy_cbs.unload(&proxy->public, &plugin->public, proxy->cb_data); + proxy->proxy_cbs.unload(&proxy->public, &plugin->public, plugin->proxy_data, proxy->cb_data); err: g_free(plugin->filename); g_free(plugin); @@ -758,6 +777,40 @@ static void remove_sources(Plugin *plugin) }
+/* Make the GModule backing plugin resident (if it's GModule-backed at all) */ +void plugin_make_resident(Plugin *plugin) +{ + if (plugin->proxy == &builtin_so_proxy_plugin) + { + g_return_if_fail(plugin->proxy_data != NULL); + g_module_make_resident(plugin->proxy_data); + } + else + g_warning("Skipping g_module_make_resident() for non-native plugin"); +} + + +/* Retrieve the address of a symbol sym located in plugin, if it's GModule-backed */ +gpointer plugin_get_module_symbol(Plugin *plugin, const gchar *sym) +{ + gpointer symbol; + + if (plugin->proxy == &builtin_so_proxy_plugin) + { + g_return_val_if_fail(plugin->proxy_data != NULL, NULL); + if (g_module_symbol(plugin->proxy_data, sym, &symbol)) + return symbol; + else + g_warning("Failed to locate signal handler for '%s': %s", + sym, g_module_error()); + } + else /* TODO: Could possibly support this via a new proxy hook */ + g_warning("Failed to locate signal handler for '%s': Not supported for non-native plugins", + sym); + return NULL; +} + + static gboolean is_active_plugin(Plugin *plugin) { return (g_list_find(active_plugin_list, plugin) != NULL); @@ -812,7 +865,10 @@ plugin_free(Plugin *plugin) active_plugin_list = g_list_remove(active_plugin_list, plugin); plugin_list = g_list_remove(plugin_list, plugin);
- proxy->proxy_cbs.unload(&proxy->public, &plugin->public, proxy->cb_data); + /* cb_data_destroy might be plugin code and must be called before unloading the module. */ + if (plugin->cb_data_destroy) + plugin->cb_data_destroy(plugin->cb_data); + proxy->proxy_cbs.unload(&proxy->public, &plugin->public, plugin->proxy_data, proxy->cb_data);
g_free(plugin->filename); g_free(plugin);
Modified: src/pluginutils.c 11 lines changed, 2 insertions(+), 9 deletions(-) =================================================================== @@ -96,8 +96,7 @@ GEANY_API_SYMBOL void plugin_module_make_resident(GeanyPlugin *plugin) { g_return_if_fail(plugin); - - g_module_make_resident(plugin->priv->module); + plugin_make_resident(plugin->priv); }
@@ -444,12 +443,7 @@ static void connect_plugin_signals(GtkBuilder *builder, GObject *object, gpointer symbol = NULL; struct BuilderConnectData *data = user_data;
- if (!g_module_symbol(data->plugin->priv->module, handler_name, &symbol)) - { - g_warning("Failed to locate signal handler for '%s': %s", - signal_name, g_module_error()); - return; - } + symbol = plugin_get_module_symbol(data->plugin->priv, handler_name);
plugin_signal_connect(data->plugin, object, signal_name, FALSE, G_CALLBACK(symbol) /*ub?*/, data->user_data); @@ -503,7 +497,6 @@ void plugin_builder_connect_signals(GeanyPlugin *plugin, struct BuilderConnectData data = { NULL };
g_return_if_fail(plugin != NULL && plugin->priv != NULL); - g_return_if_fail(plugin->priv->module != NULL); g_return_if_fail(GTK_IS_BUILDER(builder));
data.user_data = user_data;
-------------- This E-Mail was brought to you by github_commit_mail.py (Source: https://github.com/geany/infrastructure).