From 437837d3a54367393c41d6c1e1f4d1af4481627e Mon Sep 17 00:00:00 2001 From: Thomas Martitz Date: Sun, 23 Aug 2015 15:23:37 +0200 Subject: [PATCH] plugins: separate geany_plugin_set_data() dual-use It was found that because geany_plugin_set_data() could be used by both plugin's init() and geany_load_module(), that it introduced some uncertainty as to when to call the free_func. init() callers might expect the call around the same time as cleanup() is called, while geany_load_module() callers expected the call at module unload time. It was indeed called at module unload time. But that means that init() callers cannot call it again reliably after in a init()->cleanup()->init() flow (when toggling the plugin) without fully unloading the plugin (which is what we do currently but that's we would want to change). With the separation we can actually destroy the data depending on where it was set and do everything unambigiously. --- doc/plugins.dox | 37 +++++++++++++++++----------------- src/plugindata.h | 20 +++++++++++++++--- src/pluginprivate.h | 2 ++ src/plugins.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++ src/pluginutils.c | 15 +++++++++----- 5 files changed, 106 insertions(+), 26 deletions(-) diff --git a/doc/plugins.dox b/doc/plugins.dox index 3e7c9b7ca..709ccce47 100644 --- a/doc/plugins.dox +++ b/doc/plugins.dox @@ -238,20 +238,22 @@ number of steps: Manager, and @a cleanup is called on exit and when the user deactivates it. So use these to do advanced initialization and teardown as to not waste resources when the plugin is not even enabled. - 3. Actually registering by calling GEANY_PLUGIN_REGISTER(), passing the GeanyPlugin pointer that - you received and filled out as above. GEANY_PLUGIN_REGISTER() also takes the minimum API - version number you want to support (see @ref versions for details). Please also check the - return value. Geany may refuse to load your plugin due to + 3. Actually registering by calling GEANY_PLUGIN_REGISTER() or GEANY_PLUGIN_REGISTER_FULL(). + - Usually you should use GEANY_PLUGIN_REGISTER() to register your plugin, passing the + GeanyPlugin pointer that you received and filled out as above. GEANY_PLUGIN_REGISTER() also + takes the minimum API version number you want to support (see @ref versions for details). Please + also check the return value. Geany may refuse to load your plugin due to incompatibilities, you should then abort any extra setup. GEANY_PLUGIN_REGISTER() is a macro - wrapping geany_plugin_register() which takes additional the API and ABI that you should not - pass manually. - 4. Optionally setting user data that is passed to GeanyPlugin::funcs with geany_plugin_set_data(). - Here you can set a data pointer that is passed back to your functions called by Geany. This - allows to avoid global variables which may be useful. It also allows to set instance pointer - to objects in case your plugin isn't written in pure C, enabling you to use member functions - as plugin functions. - You may also call this function later on, for example in your @ref GeanyPluginFuncs::init - routine to defer costly allocations to when the plugin is actually activated by the user. + wrapping geany_plugin_register() which takes additional the API and ABI that you should not pass + manually. + - If you require a plugin-specific context or state to be passed to your GeanyPlugin::funcs then + use GEANY_PLUGIN_REGISTER_FULL() to register. This one takes additional parameters for adding + user data to your plugin. That user data pointer is subsequently passed back to your functions. + It allows, for example, to set instance pointer to objects in case your plugin isn't written in + pure C, enabling you to use member functions as plugin functions. You may also set such data + later on, for example in your @ref GeanyPluginFuncs::init routine to defer costly allocations + to when the plugin is actually activated by the user. However, you then have to call + geany_plugin_set_data(). @subsection versions On API and ABI Versions @@ -337,9 +339,8 @@ void geany_load_module(GeanyPlugin *plugin) /* Step 3: Register! */ if (GEANY_PLUGIN_REGISTER(plugin, 225)) return; - - /* Step 4: call geany_plugin_set_data(), not done here - geany_plugin_set_data(plugin, data, free_func); */ + /* alternatively: + GEANY_PLUGIN_REGISTER_FULL(plugin, 225, data, free_func); */ } @endcode @@ -359,8 +360,8 @@ extern "C" void geany_load_module(GeanyPlugin *plugin) @endcode You can also create an instance of a class and set that as data pointer (with -geany_plugin_set_data()). With small wrappers that shuffle the parameters you can even use member -functions for @ref GeanyPlugin::funcs etc. +GEANY_PLUGIN_REGISTER_FULL()). With small wrappers that shuffle the parameters you can even use +member functions for @ref GeanyPlugin::funcs etc. @section building Building diff --git a/src/plugindata.h b/src/plugindata.h index 5547556d4..9832b73d2 100644 --- a/src/plugindata.h +++ b/src/plugindata.h @@ -310,9 +310,12 @@ struct GeanyPluginFuncs void (*cleanup) (GeanyPlugin *plugin, gpointer pdata); }; -gboolean geany_plugin_register(GeanyPlugin *plugin, gint api_version, gint min_api_version, - gint abi_version); -void geany_plugin_set_data(GeanyPlugin *plugin, gpointer data, GDestroyNotify destroy_notify); +gboolean geany_plugin_register(GeanyPlugin *plugin, gint api_version, + gint min_api_version, gint abi_version); +gboolean geany_plugin_register_full(GeanyPlugin *plugin, gint api_version, + gint min_api_version, gint abi_version, + gpointer data, GDestroyNotify free_func); +void geany_plugin_set_data(GeanyPlugin *plugin, gpointer data, GDestroyNotify free_func); /** Convinience macro to register a plugin. * @@ -325,6 +328,17 @@ void geany_plugin_set_data(GeanyPlugin *plugin, gpointer data, GDestroyNotify de geany_plugin_register((plugin), GEANY_API_VERSION, \ (min_api_version), GEANY_ABI_VERSION) +/** Convinience macro to register a plugin with data. + * + * It simply calls geany_plugin_register_full() with GEANY_API_VERSION and GEANY_ABI_VERSION. + * + * @since 1.26 (API 225) + * @see @ref howto + **/ +#define GEANY_PLUGIN_REGISTER_FULL(plugin, min_api_version, pdata, free_func) \ + geany_plugin_register_full((plugin), GEANY_API_VERSION, \ + (min_api_version), GEANY_ABI_VERSION, (pdata), (free_func)) + /* Deprecated aliases */ #ifndef GEANY_DISABLE_DEPRECATED diff --git a/src/pluginprivate.h b/src/pluginprivate.h index afc1f36b1..74a6a0615 100644 --- a/src/pluginprivate.h +++ b/src/pluginprivate.h @@ -42,6 +42,7 @@ SignalConnection; typedef enum _LoadedFlags { LOADED_OK = 0x01, IS_LEGACY = 0x02, + LOAD_DATA = 0x04, } LoadedFlags; @@ -70,6 +71,7 @@ GeanyPluginPrivate; #define PLUGIN_LOADED_OK(p) (((p)->flags & LOADED_OK) != 0) #define PLUGIN_IS_LEGACY(p) (((p)->flags & IS_LEGACY) != 0) +#define PLUGIN_HAS_LOAD_DATA(p) (((p)->flags & LOAD_DATA) != 0) typedef GeanyPluginPrivate Plugin; /* shorter alias */ diff --git a/src/plugins.c b/src/plugins.c index bad251d67..bcc42e9b9 100644 --- a/src/plugins.c +++ b/src/plugins.c @@ -297,6 +297,9 @@ gboolean geany_plugin_register(GeanyPlugin *plugin, gint api_version, gint min_a g_return_val_if_fail(plugin != NULL, FALSE); p = plugin->priv; + /* already registered successfully */ + g_return_val_if_fail(!PLUGIN_LOADED_OK(p), FALSE); + /* Prevent registering incompatible plugins. */ if (! plugin_check_version(p, PLUGIN_VERSION_CODE(api_version, abi_version))) return FALSE; @@ -322,6 +325,51 @@ gboolean geany_plugin_register(GeanyPlugin *plugin, gint api_version, gint min_a return PLUGIN_LOADED_OK(p); } + +/** Register a plugin to Geany, with plugin-defined data. + * + * This is a variant of geany_plugin_register() that also allows to set the plugin-defined data. + * Refer to that function for more details on registering in general. + * + * @p pdata is the pointer going to be passed to the individual plugin callbacks + * of GeanyPlugin::funcs. When the plugin module is unloaded, @p free_func is invoked on + * @p pdata, which connects the data to the plugin's module life time. + * + * You cannot use geany_plugin_set_data() after registering with this function. Use + * geany_plugin_register() if you need to. + * + * Do not call this directly. Use GEANY_PLUGIN_REGISTER_FULL() instead which automatically + * handles @p api_version and @p abi_version. + * + * @param plugin The plugin provided by Geany. + * @param api_version The API version the plugin is compiled against (pass GEANY_API_VERSION). + * @param min_api_version The minimum API version required by the plugin. + * @param abi_version The exact ABI version the plugin is compiled against (pass GEANY_ABI_VERSION). + * @param pdata Pointer to the plugin-defined data. Must not be @c NULL. + * @param free_func Function used to deallocate @a pdata, may be @c NULL. + * + * @return TRUE if the plugin was successfully registered. Otherwise FALSE. + * + * @since 1.26 (API 225) + * @see GEANY_PLUGIN_REGISTER_FULL() + * @see geany_plugin_register() + **/ +GEANY_API_SYMBOL +gboolean geany_plugin_register_full(GeanyPlugin *plugin, gint api_version, gint min_api_version, + gint abi_version, gpointer pdata, GDestroyNotify free_func) +{ + if (geany_plugin_register(plugin, api_version, min_api_version, abi_version)) + { + geany_plugin_set_data(plugin, pdata, free_func); + /* We use LOAD_DATA to indicate that pdata cb_data was set during loading/registration + * as opposed to during GeanyPluginFuncs::init(). In the latter case we call free_func + * after GeanyPluginFuncs::cleanup() */ + plugin->priv->flags |= LOAD_DATA; + return TRUE; + } + return FALSE; +} + struct LegacyRealFuncs { void (*init) (GeanyData *data); @@ -689,6 +737,16 @@ plugin_cleanup(Plugin *plugin) if (widget) gtk_widget_destroy(widget); + if (!PLUGIN_HAS_LOAD_DATA(plugin) && plugin->cb_data_destroy) + { + /* If the plugin has used geany_plugin_set_data(), destroy the data here. But don't + * if it was already set through geany_plugin_register_full() because we couldn't call + * its init() anymore (not without completely reloading it anyway). */ + plugin->cb_data_destroy(plugin->cb_data); + plugin->cb_data = NULL; + plugin->cb_data_destroy = NULL; + } + geany_debug("Unloaded: %s", plugin->filename); } diff --git a/src/pluginutils.c b/src/pluginutils.c index 2386c667c..782498c0b 100644 --- a/src/pluginutils.c +++ b/src/pluginutils.c @@ -515,10 +515,9 @@ void plugin_builder_connect_signals(GeanyPlugin *plugin, /** Add additional data that corresponds to the plugin. * - * This is the data pointer passed to the individual plugin callbacks. It may be - * called as soon as geany_plugin_register() was called with success. When the - * plugin is unloaded, @a free_func is invoked for the data, which connects the - * data to the plugin's own life time. + * @p pdata is the pointer going to be passed to the individual plugin callbacks + * of GeanyPlugin::funcs. When the plugin is cleaned up, @p free_func is invoked for the data, + * which connects the data to the time the plugin is enabled. * * One intended use case is to set GObjects as data and have them destroyed automatically * by passing g_object_unref() as @a free_func, so that member functions can be used @@ -527,6 +526,9 @@ void plugin_builder_connect_signals(GeanyPlugin *plugin, * Be aware that this can only be called once and only by plugins registered via * @ref geany_plugin_register(). So-called legacy plugins cannot use this function. * + * @note This function must not be called if the plugin was registered with + * geany_plugin_register_full(). + * * @param plugin The plugin provided by Geany * @param pdata The plugin's data to associate, must not be @c NULL * @param free_func The destroy notify @@ -552,7 +554,10 @@ void geany_plugin_set_data(GeanyPlugin *plugin, gpointer pdata, GDestroyNotify f * wrong one that can only be replaced by another one. */ if (p->cb_data != NULL || p->cb_data_destroy != NULL) { - g_warning("Double call to %s(), ignored!", G_STRFUNC); + if (PLUGIN_HAS_LOAD_DATA(p)) + g_warning("Invalid call to %s(), geany_plugin_register_full() was used. Ignored!\n", G_STRFUNC); + else + g_warning("Double call to %s(), ignored!", G_STRFUNC); return; } -- 2.11.4.GIT