Remove support for external plugins

From: Emil Velikov <[email protected]>

As a whole all plugins should be built-in, otherwise they would be using internal, undocumented, unversioned, unstable API.

Flesh out the external plugin support into a few pre-processor blocks and simplify the normal path.

Hide the internal API (omit export-dynamic) when built without external plugins.

---
 Makefile.obexd     |  2 ++
 obexd/src/obexd.h  |  2 +-
 obexd/src/plugin.c | 93 ++++++++++++++++++++++++++++++++++++------------------
 obexd/src/plugin.h |  4 +++
 4 files changed, 70 insertions(+), 31 deletions(-)

Makefile.obexd +2 -0

index 5d1a4ff65..9175de2a4 100644
@@ -86,7 +86,9 @@ obexd_src_obexd_LDADD = lib/libbluetooth-internal.la \
                        $(ICAL_LIBS) $(DBUS_LIBS) $(LIBEBOOK_LIBS) \
                        $(LIBEDATASERVER_LIBS) $(GLIB_LIBS) -ldl
 
+if EXTERNAL_PLUGINS
 obexd_src_obexd_LDFLAGS = $(AM_LDFLAGS) -Wl,--export-dynamic
+endif
 
 obexd_src_obexd_CPPFLAGS = $(AM_CPPFLAGS) $(GLIB_CFLAGS) $(DBUS_CFLAGS) \
                                $(ICAL_CFLAGS) -DOBEX_PLUGIN_BUILTIN \

obexd/src/obexd.h +1 -1

index fe312a65b..af5265da5 100644
@@ -18,7 +18,7 @@
 #define OBEX_MAS       (1 << 8)
 #define OBEX_MNS       (1 << 9)
 
-gboolean plugin_init(const char *pattern, const char *exclude);
+void plugin_init(const char *pattern, const char *exclude);
 void plugin_cleanup(void);
 
 gboolean manager_init(void);

obexd/src/plugin.c +63 -30

index a3eb24753..212299fa5 100644
@@ -37,11 +37,14 @@
 static GSList *plugins = NULL;
 
 struct obex_plugin {
+#if EXTERNAL_PLUGINS
        void *handle;
+#endif
        const struct obex_plugin_desc *desc;
 };
 
-static gboolean add_plugin(void *handle, const struct obex_plugin_desc *desc)
+#if EXTERNAL_PLUGINS
+static gboolean add_external_plugin(void *handle, const struct obex_plugin_desc *desc)
 {
        struct obex_plugin *plugin;
 

@@ -65,6 +68,26 @@ static gboolean add_plugin(void *handle, const struct obex_plugin_desc *desc)
 
        return TRUE;
 }
+#endif
+
+static void add_plugin(const struct obex_plugin_desc *desc)
+{
+       struct obex_plugin *plugin;
+
+       plugin = g_try_new0(struct obex_plugin, 1);
+       if (plugin == NULL)
+               return;
+
+       plugin->desc = desc;
+
+       if (desc->init() < 0) {
+               g_free(plugin);
+               return;
+       }
+
+       plugins = g_slist_append(plugins, plugin);
+       DBG("Plugin %s loaded", desc->name);
+}
 
 static gboolean check_plugin(const struct obex_plugin_desc *desc,
                                char **patterns, char **excludes)

@@ -93,42 +116,23 @@ static gboolean check_plugin(const struct obex_plugin_desc *desc,
 }
 
 
-#include "builtin.h"
-
-gboolean plugin_init(const char *pattern, const char *exclude)
+static void external_plugin_init(char **patterns, char **excludes)
 {
-       char **patterns = NULL;
-       char **excludes = NULL;
+#if EXTERNAL_PLUGINS
        GDir *dir;
        const char *file;
-       unsigned int i;
 
-       if (strlen(PLUGINDIR) == 0)
-               return FALSE;
-
-       if (pattern)
-               patterns = g_strsplit_set(pattern, ":, ", -1);
-
-       if (exclude)
-               excludes = g_strsplit_set(exclude, ":, ", -1);
-
-       DBG("Loading builtin plugins");
-
-       for (i = 0; __obex_builtin[i]; i++) {
-               if (check_plugin(__obex_builtin[i],
-                                       patterns, excludes) == FALSE)
-                       continue;
+       warn("Using external plugins is not officially supported.\n");
+       warn("Consider upstreaming your plugins into the BlueZ project.");
 
-               add_plugin(NULL,  __obex_builtin[i]);
-       }
+       if (strlen(PLUGINDIR) == 0)
+               return;
 
        DBG("Loading plugins %s", PLUGINDIR);
 
        dir = g_dir_open(PLUGINDIR, 0, NULL);
        if (!dir) {
-               g_strfreev(patterns);
-               g_strfreev(excludes);
-               return FALSE;
+               return;
        }
 
        while ((file = g_dir_read_name(dir)) != NULL) {

@@ -164,15 +168,42 @@ gboolean plugin_init(const char *pattern, const char *exclude)
                        continue;
                }
 
-               if (add_plugin(handle, desc) == FALSE)
+               if (add_external_plugin(handle, desc) == FALSE)
                        dlclose(handle);
        }
 
        g_dir_close(dir);
+#endif
+}
+
+#include "builtin.h"
+
+void plugin_init(const char *pattern, const char *exclude)
+{
+       char **patterns = NULL;
+       char **excludes = NULL;
+       unsigned int i;
+
+       if (pattern)
+               patterns = g_strsplit_set(pattern, ":, ", -1);
+
+       if (exclude)
+               excludes = g_strsplit_set(exclude, ":, ", -1);
+
+       DBG("Loading builtin plugins");
+
+       for (i = 0; __obex_builtin[i]; i++) {
+               if (check_plugin(__obex_builtin[i],
+                                       patterns, excludes) == FALSE)
+                       continue;
+
+               add_plugin(__obex_builtin[i]);
+       }
+
+       external_plugin_init(patterns, excludes);
+
        g_strfreev(patterns);
        g_strfreev(excludes);
-
-       return TRUE;
 }
 
 void plugin_cleanup(void)

@@ -187,8 +218,10 @@ void plugin_cleanup(void)
                if (plugin->desc->exit)
                        plugin->desc->exit();
 
+#if EXTERNAL_PLUGINS
                if (plugin->handle != NULL)
                        dlclose(plugin->handle);
+#endif
 
                g_free(plugin);
        }

obexd/src/plugin.h +4 -1

index a91746cbc..e1756b9bf 100644
@@ -20,10 +20,14 @@ struct obex_plugin_desc {
                        #name, init, exit \
                };
 #else
+#if EXTERNAL_PLUGINS
 #define OBEX_PLUGIN_DEFINE(name,init,exit) \
                extern struct obex_plugin_desc obex_plugin_desc \
                                __attribute__ ((visibility("default"))); \
                const struct obex_plugin_desc obex_plugin_desc = { \
                        #name, init, exit \
                };
+#else
+#error "Requested non built-in plugin, while external plugins is disabled"
+#endif
 #endif
 
--

2.43.0

https://gist.github.com/Vudentz/0b8bcb78a8898614fc4848cbf44a0a9f

That way we leave it to the compiler to remove the dead code when linking but it still build checks which catches errors such as:

Hello everyone,

Here's v3 fixing a small bug with the previous patches, which was tripping the CI.

Link to the previous revision can be found below.

Thanks
Emil

- Link to v2: https://lore.kernel.org/r/[email protected]

---
Emil Velikov (8):
configure, README: introduce --enable-external-plugins
obexd: factor out external plugin support
bluetoothd: remove external-dummy plugin
bluetoothd: convert external sixaxis plugin to builtin
bluetoothd: factor out external plugin support
bluetoothd: don't export internal API
bluetoothd: change plugin loading alike obexd
android: export only (android) entrypoint from the modules

 Makefile.am              |  17 ++----
 Makefile.obexd           |   2 +
 Makefile.plugins         |   8 +--
 README                   |  13 +++++
 android/Makefile.am      |   3 +
 android/hal-audio.c      |   1 +
 android/hal-bluetooth.c  |   1 +
 android/hal-sco.c        |   1 +
 configure.ac             |  10 ++++
 obexd/src/obexd.h        |   2 +-
 obexd/src/plugin.c       |  93 +++++++++++++++++++++----------
 obexd/src/plugin.h       |   4 ++
 plugins/external-dummy.c |  28 ----------
 src/btd.h                |   2 +-
 src/plugin.c             | 140 +++++++++++++++++++++++++++++------------------
 src/plugin.h             |  16 ++++++
 16 files changed, 209 insertions(+), 132 deletions(-)

---
base-commit: a9d1f6f6a625607de6c3f5b7a40a3aac5f36c02b
change-id: 20240116-rm-ext-plugins-ba0b852a492b

Best regards,
--
Emil Velikov <[email protected]>

Hi,

Quoted 6 lines · On Thu, 25 Jan 2024 at 01:07, Emil Velikov via B4 Relay…

Hello everyone,

Here's v3 fixing a small bug with the previous patches, which was tripping the CI.

Quoted 1 line

Link to the previous revision can be found below.

Just a comment that external plugins support was added to avoid udev dependency (from sixaxis) in bluetoothd.
(not that I have strong opinion on this, just a note, I don't remember exactly why it was done, maybe Marcel recalls?)

Quoted 11 lines · Szymon Janc

Hi,

Quoted 6 lines · On Thu, 25 Jan 2024 at 01:07, Emil Velikov via B4 Relay…

Hello everyone,

Here's v3 fixing a small bug with the previous patches, which was tripping the CI.

Quoted 7 lines

Quoted 1 line

Link to the previous revision can be found below.

Just a comment that external plugins support was added to avoid udev dependency (from sixaxis) in bluetoothd.
(not that I have strong opinion on this, just a note, I don't remember exactly why it was done, maybe Marcel recalls?)

Thanks, I may have some ideas why.

About 10 years ago (or so) some distributions were shipping libudev.so.0 while others libudev.so.1. The ABI break was minimal, although it was a thing.
I remember us doing all sorts of hacks in Mesa trying to pick the correct one, esp when your system can have .1 while the game (or its chroot-like environment) has .0 and vice-versa.

I would imagine a similar issue was observed in bluez - but I can only speculate.

Over the last 5+ years, literally all supported distributions have moved for libudev.so.1 and the Steam games (and runtime) has both with some compat quirks to avoid explosions.

Hi Emil,

Quoted 33 lines · Emil Velikov

Quoted 11 lines · Szymon Janc

Hi,

Quoted 6 lines · On Thu, 25 Jan 2024 at 01:07, Emil Velikov via B4 Relay…

Hello everyone,

Here's v3 fixing a small bug with the previous patches, which was tripping the CI.

Quoted 7 lines

Quoted 1 line

Link to the previous revision can be found below.

Just a comment that external plugins support was added to avoid udev dependency (from sixaxis) in bluetoothd.
(not that I have strong opinion on this, just a note, I don't remember exactly why it was done, maybe Marcel recalls?)

Thanks, I may have some ideas why.

About 10 years ago (or so) some distributions were shipping libudev.so.0 while others libudev.so.1. The ABI break was minimal, although it was a thing.
I remember us doing all sorts of hacks in Mesa trying to pick the correct one, esp when your system can have .1 while the game (or its chroot-like environment) has .0 and vice-versa.

I would imagine a similar issue was observed in bluez - but I can only speculate.

Quoted 3 lines

Over the last 5+ years, literally all supported distributions have moved for libudev.so.1 and the Steam games (and runtime) has both with some compat quirks to avoid explosions.

I was considering applying this week but if you want to respin this set to sort out the dependency Im fine with, but I don't think it would hurt to have a libudev dependency provide we have some means to disable it in case the system don't intend to support sixaxis plugin.