Refactor plugin registration and loading

Put different types of plugins (libwiretap, libwireshark) in different
subdirectories, give libwiretap and libwireshark init routines that
load the plugins, and have them scan the appropriate subdirectories
so that we don't even *try* to, for example, load libwireshark plugins
in programs that only use libwiretap.

Compiled plugins are stored in subfolders of the plugin folders, with
the subfolder name being the Wireshark minor version number (X.Y). There is
another hierarchical level for each Wireshark library (libwireshark, libwscodecs
and libwiretap).

The folder names are respectively plugins/X.Y/{epan,codecs,wiretap}.

Currently we only distribute "epan" (libwireshark) plugins.

Change-Id: I3438787a6f45820d64ba4ca91cbe3c8864708acb
Reviewed-on: https://code.wireshark.org/review/23983
Petri-Dish: João Valverde <j@v6e.pt>
Tested-by: Petri Dish Buildbot
Reviewed-by: João Valverde <j@v6e.pt>

2 files changed, 150 insertions, 233 deletions

diff --git a/wsutil/plugins.c b/wsutil/plugins.c
index 503889ae71..c4ef3f26ce 100644
--- a/wsutil/plugins.c
+++ b/wsutil/plugins.c
@@ -34,218 +34,140 @@ typedef struct _plugin {
     GModule        *handle;       /* handle returned by g_module_open */
     gchar          *name;         /* plugin name */
     const gchar    *version;      /* plugin version */
-    GString        *types;        /* description with types this plugin supports */
+    const gchar    *type;         /* type of plugin */
 } plugin;
 
+/* array of plugins */
+static GPtrArray *plugins_array = NULL;
+/* map of names to plugin */
 static GHashTable *plugins_table = NULL;
 
 static void
-free_plugin(gpointer _p)
+free_plugin(gpointer data)
 {
-    plugin *p = (plugin *)_p;
+    plugin *p = (plugin *)data;
     g_module_close(p->handle);
     g_free(p->name);
-    g_string_free(p->types, TRUE);
     g_free(p);
 }
 
-/*
- * Add a new plugin type.
- * Takes a callback routine as an argument; it is called for each plugin
- * we find, and handed a handle for the plugin, the name of the plugin,
- * and the version string for the plugin.  The plugin returns TRUE if
- * it's a plugin for that type and FALSE if not.
- */
-typedef struct {
-    const char *name;
-    plugin_check_type_callback check_type;
-} plugin_type;
-
-static GSList *plugin_types = NULL;
-
-static void
-free_plugin_type(gpointer p, gpointer user_data _U_)
+static gint
+compare_plugins(gconstpointer a, gconstpointer b)
 {
-    g_free(p);
-}
-
-void
-add_plugin_type(const char *name, plugin_check_type_callback check_type)
-{
-    plugin_type *new_type;
-
-    new_type = (plugin_type *)g_malloc(sizeof (plugin_type));
-    new_type->name = name;
-    new_type->check_type = check_type;
-    plugin_types = g_slist_prepend(plugin_types, new_type);
+    return g_strcmp0((*(const plugin **)a)->name, (*(const plugin **)b)->name);
 }
 
 static void
-call_plugin_callback(gpointer data, gpointer user_data)
+plugins_scan_dir(GPtrArray **plugins_ptr, const char *dirpath, const char *type_name, gboolean build_dir)
 {
-    plugin_type *type = (plugin_type *)data;
-    plugin *new_plug = (plugin *)user_data;
-
-    if (type->check_type(new_plug->handle)) {
-        /* The plugin supports this type */
-        if (new_plug->types->len > 0)
-            g_string_append(new_plug->types, ", ");
-        g_string_append(new_plug->types, type->name);
-    }
-}
-
-static void
-plugins_scan_dir(const char *dirname, plugin_load_failure_mode mode)
-{
-    WS_DIR        *dir;             /* scanned directory */
-    WS_DIRENT     *file;            /* current file */
-    const char    *name;
-    gchar         *filename;        /* current file name */
+    GDir          *dir;
+    const char    *name;            /* current file name */
+    gchar         *path;            /* current file full path */
     GModule       *handle;          /* handle returned by g_module_open */
     gpointer       symbol;
     const char    *plug_version, *plug_release;
     plugin        *new_plug;
     gchar         *dot;
 
-    if (!g_file_test(dirname, G_FILE_TEST_EXISTS) || !g_file_test(dirname, G_FILE_TEST_IS_DIR)) {
+    dir = g_dir_open(dirpath, 0, NULL);
+    if (dir == NULL)
         return;
-    }
 
-    if ((dir = ws_dir_open(dirname, 0, NULL)) != NULL)
-    {
-        while ((file = ws_dir_read_name(dir)) != NULL)
-        {
-            name = ws_dir_get_name(file);
-
-            /*
-             * GLib 2.x defines G_MODULE_SUFFIX as the extension used on
-             * this platform for loadable modules.
-             */
-            /* skip anything but files with G_MODULE_SUFFIX */
-            dot = strrchr(name, '.');
-            if (dot == NULL || strcmp(dot+1, G_MODULE_SUFFIX) != 0)
-                continue;
+    while ((name = g_dir_read_name(dir)) != NULL) {
+        /* Skip anything but files with G_MODULE_SUFFIX. */
+        dot = strrchr(name, '.');
+        if (dot == NULL || strcmp(dot+1, G_MODULE_SUFFIX) != 0)
+            continue;
 
 #if WIN32
-            if (strncmp(name, "nordic_ble.dll", 14) == 0)
-                /*
-                 * Skip the Nordic BLE Sniffer dll on WIN32 because
-                 * the dissector has been added as internal.
-                 */
-                continue;
-#endif
-
+        if (strncmp(name, "nordic_ble.dll", 14) == 0) {
             /*
-             * Check if the same name is already registered.
+             * Skip the Nordic BLE Sniffer dll on WIN32 because
+             * the dissector has been added as internal.
              */
-            if (g_hash_table_lookup(plugins_table, name)) {
-                /* Yes, it is. */
-                if (mode == REPORT_LOAD_FAILURE) {
-                    report_warning("The plugin '%s' was found "
-                            "in multiple directories", name);
-                }
-                continue;
-            }
-
-            filename = g_build_filename(dirname, name, (gchar *)NULL);
-            handle = g_module_open(filename, G_MODULE_BIND_LOCAL);
-            g_free(filename);
-            if (handle == NULL)
-            {
-                /*
-                 * Only report load failures if we were asked to.
-                 *
-                 * XXX - we really should put different types of plugins
-                 * (libwiretap, libwireshark) in different subdirectories,
-                 * give libwiretap and libwireshark init routines that
-                 * load the plugins, and have them scan the appropriate
-                 * subdirectories so tha we don't even *try* to, for
-                 * example, load libwireshark plugins in programs that
-                 * only use libwiretap.
-                 */
-                if (mode == REPORT_LOAD_FAILURE) {
-                    /* g_module_error() provides filename. */
-                    report_failure("Couldn't load plugin '%s': %s", name,
-                                   g_module_error());
-                }
-                continue;
-            }
-
-            if (!g_module_symbol(handle, "plugin_version", &symbol))
-            {
-                report_failure("The plugin '%s' has no \"plugin_version\" symbol", name);
-                g_module_close(handle);
-                continue;
-            }
-            plug_version = (const char *)symbol;
+            continue;
+        }
+#endif
 
-            if (!g_module_symbol(handle, "plugin_release", &symbol))
-            {
-                report_failure("The plugin '%s' has no \"plugin_release\" symbol", name);
-                g_module_close(handle);
-                continue;
-            }
-            plug_release = (const char *)symbol;
-            if (strcmp(plug_release, VERSION_RELEASE) != 0) {
-                report_failure("The plugin '%s' was compiled for Wireshark version %s", name, plug_release);
-                g_module_close(handle);
-                continue;
+        /*
+         * Check if the same name is already registered.
+         */
+        if (g_hash_table_lookup(plugins_table, name)) {
+            /* Yes, it is. */
+            if (!build_dir) {
+                report_warning("The plugin '%s' was found "
+                                "in multiple directories", name);
             }
+            continue;
+        }
 
-            new_plug = (plugin *)g_malloc(sizeof(plugin));
-            new_plug->handle = handle;
-            new_plug->name = g_strdup(name);
-            new_plug->version = plug_version;
-            new_plug->types = g_string_new(NULL);
+        path = g_build_filename(dirpath, name, (gchar *)NULL);
+        handle = g_module_open(path, G_MODULE_BIND_LOCAL);
+        g_free(path);
+        if (handle == NULL) {
+            /* g_module_error() provides file path. */
+            report_failure("Couldn't load plugin '%s': %s", name,
+                            g_module_error());
+            continue;
+        }
 
-            /*
-             * Hand the plugin to each of the plugin type callbacks.
-             */
-            g_slist_foreach(plugin_types, call_plugin_callback, new_plug);
+        if (!g_module_symbol(handle, "plugin_version", &symbol))
+        {
+            report_failure("The plugin '%s' has no \"plugin_version\" symbol", name);
+            g_module_close(handle);
+            continue;
+        }
+        plug_version = (const char *)symbol;
 
-            /*
-             * Does this dissector do anything useful?
-             */
-            if (new_plug->types->len == 0)
-            {
-                /*
-                 * No.
-                 *
-                 * Only report this failure if we were asked to; it might
-                 * just mean that it's a plugin type that this program
-                 * doesn't support, such as a libwireshark plugin in
-                 * a program that doesn't use libwireshark.
-                 *
-                 * XXX - we really should put different types of plugins
-                 * (libwiretap, libwireshark) in different subdirectories,
-                 * give libwiretap and libwireshark init routines that
-                 * load the plugins, and have them scan the appropriate
-                 * subdirectories so tha we don't even *try* to, for
-                 * example, load libwireshark plugins in programs that
-                 * only use libwiretap.
-                 */
-                if (mode == REPORT_LOAD_FAILURE) {
-                    report_failure("The plugin '%s' has no registration routines",
-                                   name);
-                }
-                free_plugin(new_plug);
-                continue;
-            }
+        if (!g_module_symbol(handle, "plugin_release", &symbol))
+        {
+            report_failure("The plugin '%s' has no \"plugin_release\" symbol", name);
+            g_module_close(handle);
+            continue;
+        }
+        plug_release = (const char *)symbol;
+        if (strcmp(plug_release, VERSION_RELEASE) != 0) {
+            report_failure("The plugin '%s' was compiled for Wireshark version %s", name, plug_release);
+            g_module_close(handle);
+            continue;
+        }
 
-            /*
-             * OK, add it to the list of plugins.
-             */
-            g_hash_table_insert(plugins_table, new_plug->name, new_plug);
+        /* Search for the entry point for the plugin type */
+        if (!g_module_symbol(handle, "plugin_register", &symbol)) {
+            report_failure("The plugin '%s' has no \"plugin_register\" symbol", name);
+            g_module_close(handle);
+            continue;
         }
-        ws_dir_close(dir);
+
+DIAG_OFF(pedantic)
+        /* Found it, call the plugin registration function. */
+        ((plugin_register_func)symbol)();
+DIAG_ON(pedantic)
+
+        new_plug = (plugin *)g_malloc(sizeof(plugin));
+        new_plug->handle = handle;
+        new_plug->name = g_strdup(name);
+        new_plug->version = plug_version;
+        if (build_dir)
+            new_plug->type = "[build]";
+        else
+            new_plug->type = type_name;
+
+        /* Add it to the list of plugins. */
+        if (*plugins_ptr == NULL)
+            *plugins_ptr = g_ptr_array_new_with_free_func(free_plugin);
+        g_ptr_array_add(*plugins_ptr, new_plug);
+        g_ptr_array_add(plugins_array, new_plug);
+        g_hash_table_insert(plugins_table, new_plug->name, new_plug);
     }
+    ws_dir_close(dir);
 }
 
 /*
  * Scan the buildir for plugins.
  */
 static void
-scan_plugins_build_dir(plugin_load_failure_mode mode)
+scan_plugins_build_dir(GPtrArray **plugins_ptr, const char *type_name)
 {
     const char *plugin_dir;
     const char *name;
@@ -257,7 +179,7 @@ scan_plugins_build_dir(plugin_load_failure_mode mode)
     if ((dir = ws_dir_open(plugin_dir, 0, NULL)) == NULL)
         return;
 
-    plugins_scan_dir(plugin_dir, mode);
+    plugins_scan_dir(plugins_ptr, plugin_dir, type_name, TRUE);
     while ((file = ws_dir_read_name(dir)) != NULL)
     {
         name = ws_dir_get_name(file);
@@ -280,7 +202,7 @@ scan_plugins_build_dir(plugin_load_failure_mode mode)
             g_free(plugin_dir_path);
             plugin_dir_path = g_build_filename(plugin_dir, name, (gchar *)NULL);
         }
-        plugins_scan_dir(plugin_dir_path, mode);
+        plugins_scan_dir(plugins_ptr, plugin_dir_path, type_name, TRUE);
         g_free(plugin_dir_path);
     }
     ws_dir_close(dir);
@@ -289,16 +211,20 @@ scan_plugins_build_dir(plugin_load_failure_mode mode)
 /*
  * Scan for plugins.
  */
-void
-scan_plugins(plugin_load_failure_mode mode)
+plugins_t *
+plugins_init(const char *type_name)
 {
     if (!g_module_supported())
-        return; /* nothing to do */
+        return NULL; /* nothing to do */
+
+    gchar *dirpath;
+    GPtrArray *plugins = NULL;
 
-    if (plugins_table != NULL)
-        return; /* only scan for plugins once */
+    if (plugins_table == NULL)
+        plugins_table = g_hash_table_new(g_str_hash, g_str_equal);
+    if (plugins_array == NULL)
+        plugins_array = g_ptr_array_new();
 
-    plugins_table = g_hash_table_new_full(g_str_hash, g_str_equal, NULL, free_plugin);
     /*
      * Scan the global plugin directory.
      * If we're running from a build directory, scan the "plugins"
@@ -308,11 +234,13 @@ scan_plugins(plugin_load_failure_mode mode)
      */
     if (running_in_build_directory())
     {
-        scan_plugins_build_dir(mode);
+        scan_plugins_build_dir(&plugins, type_name);
     }
     else
     {
-        plugins_scan_dir(get_plugins_dir_with_version(), mode);
+        dirpath = g_build_filename(get_plugins_dir_with_version(), type_name, (gchar *)NULL);
+        plugins_scan_dir(&plugins, dirpath, type_name, FALSE);
+        g_free(dirpath);
     }
 
     /*
@@ -325,55 +253,26 @@ scan_plugins(plugin_load_failure_mode mode)
      */
     if (!started_with_special_privs())
     {
-        plugins_scan_dir(get_plugins_pers_dir_with_version(), mode);
+        dirpath = g_build_filename(get_plugins_pers_dir_with_version(), type_name, (gchar *)NULL);
+        plugins_scan_dir(&plugins, dirpath, type_name, FALSE);
+        g_free(dirpath);
     }
-}
-
-static void
-add_plugin_to_descriptions(gpointer key _U_, gpointer value, gpointer user_data)
-{
-    g_ptr_array_add((GPtrArray *)user_data, value);
-}
-
-static int
-compare_plugins(gconstpointer _a, gconstpointer _b)
-{
-    const plugin *a = *(const plugin **)_a;
-    const plugin *b = *(const plugin **)_b;
 
-    return strcmp(a->name, b->name);
-}
-
-struct description_callback {
-    plugin_description_callback callback;
-    gpointer callback_data;
-};
-
-static void
-call_description_callback(gpointer data, gpointer user_data)
-{
-    plugin *plug = (plugin *)data;
-    struct description_callback *desc = (struct description_callback *)user_data;
+    g_ptr_array_sort(plugins_array, compare_plugins);
 
-    desc->callback(plug->name, plug->version, plug->types->str, g_module_name(plug->handle), desc->callback_data);
+    return plugins;
 }
 
 WS_DLL_PUBLIC void
-plugins_get_descriptions(plugin_description_callback callback, void *user_data)
+plugins_get_descriptions(plugin_description_callback callback, void *callback_data)
 {
-    GPtrArray *descriptions;
-    struct description_callback cb;
-
-    if (!plugins_table)
+    if (!plugins_array)
         return;
 
-    descriptions = g_ptr_array_sized_new(g_hash_table_size(plugins_table));
-    g_hash_table_foreach(plugins_table, add_plugin_to_descriptions, descriptions);
-    g_ptr_array_sort(descriptions, compare_plugins);
-    cb.callback = callback;
-    cb.callback_data = user_data;
-    g_ptr_array_foreach(descriptions, call_description_callback, &cb);
-    g_ptr_array_free(descriptions, FALSE);
+    for (guint i = 0; i < plugins_array->len; i++) {
+        plugin *plug = (plugin *)plugins_array->pdata[i];
+        callback(plug->name, plug->version, plug->type, g_module_name(plug->handle), callback_data);
+    }
 }
 
 static void
@@ -399,12 +298,28 @@ plugins_get_count(void)
 }
 
 void
-plugins_cleanup(void)
+plugins_cleanup(plugins_t *plugins)
 {
-    if (plugins_table)
+    if (plugins)
+        g_ptr_array_free((GPtrArray *)plugins, TRUE);
+
+    /*
+     * This module uses global bookkeeping data structures and per-library
+     * objects sharing data. To avoid having to walk the plugins GPtrArray
+     * and delete each plugin from the global data structures we purge them
+     * once the first plugin cleanup function is called. This means that after
+     * calling ONE OF POSSIBLY MANY plugin cleanup function NO OTHER plugin
+     * APIs can be used except plugins_cleanup. If it ever becomes an issue
+     * it will be easy to change, for a small performance penalty.
+     */
+    if (plugins_table) {
         g_hash_table_destroy(plugins_table);
-    g_slist_foreach(plugin_types, free_plugin_type, NULL);
-    g_slist_free(plugin_types);
+        plugins_table = NULL;
+    }
+    if (plugins_array) {
+        g_ptr_array_free(plugins_array, FALSE);
+        plugins_array = NULL;
+    }
 }
 
 #endif /* HAVE_PLUGINS */
diff --git a/wsutil/plugins.h b/wsutil/plugins.h
index 2fa37c1073..c476211d92 100644
--- a/wsutil/plugins.h
+++ b/wsutil/plugins.h
@@ -20,21 +20,23 @@ extern "C" {
 
 #include "ws_symbol_export.h"
 
-typedef gboolean (*plugin_check_type_callback)(GModule *handle);
-
-typedef enum {
-    REPORT_LOAD_FAILURE,
-    DONT_REPORT_LOAD_FAILURE
-} plugin_load_failure_mode;
-WS_DLL_PUBLIC void scan_plugins(plugin_load_failure_mode mode);
-WS_DLL_PUBLIC void add_plugin_type(const char *type, plugin_check_type_callback callback);
+typedef void (*plugin_register_func)(void);
+
+typedef void plugins_t;
+
+WS_DLL_PUBLIC plugins_t *plugins_init(const char *type_name);
+
 typedef void (*plugin_description_callback)(const char *name, const char *version,
                                             const char *types, const char *filename,
                                             void *user_data);
+
 WS_DLL_PUBLIC void plugins_get_descriptions(plugin_description_callback callback, void *user_data);
+
 WS_DLL_PUBLIC void plugins_dump_all(void);
+
 WS_DLL_PUBLIC int plugins_get_count(void);
-WS_DLL_PUBLIC void plugins_cleanup(void);
+
+WS_DLL_PUBLIC void plugins_cleanup(plugins_t *plugins);
 
 #ifdef __cplusplus
 }