[geany/geany] 437837: plugins: separate geany_plugin_set_data() dual-use

Thomas Martitz git-noreply at xxxxx
Sun Aug 23 18:01:42 UTC 2015


Branch:      refs/heads/master
Author:      Thomas Martitz <kugel at rockbox.org>
Committer:   Thomas Martitz <kugel at rockbox.org>
Date:        Sun, 23 Aug 2015 18:01:42 UTC
Commit:      437837d3a54367393c41d6c1e1f4d1af4481627e
             https://github.com/geany/geany/commit/437837d3a54367393c41d6c1e1f4d1af4481627e

Log Message:
-----------
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.


Modified Paths:
--------------
    doc/plugins.dox
    src/plugindata.h
    src/pluginprivate.h
    src/plugins.c
    src/pluginutils.c

Modified: doc/plugins.dox
37 lines changed, 19 insertions(+), 18 deletions(-)
===================================================================
@@ -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 <b>check the
-    return value</b>. 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 <b>check the return value</b>. 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
 


Modified: src/plugindata.h
20 lines changed, 17 insertions(+), 3 deletions(-)
===================================================================
@@ -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
 


Modified: src/pluginprivate.h
2 lines changed, 2 insertions(+), 0 deletions(-)
===================================================================
@@ -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 */
 


Modified: src/plugins.c
58 lines changed, 58 insertions(+), 0 deletions(-)
===================================================================
@@ -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);
 }
 


Modified: src/pluginutils.c
15 lines changed, 10 insertions(+), 5 deletions(-)
===================================================================
@@ -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;
 	}
 



--------------
This E-Mail was brought to you by github_commit_mail.py (Source: https://github.com/geany/infrastructure).


More information about the Commits mailing list