| Message ID | 1312553358-26280-4-git-send-email-ospite@studenti.unina.it (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 5 Aug 2011 16:09:17 +0200 Antonio Ospite <ospite@studenti.unina.it> wrote: > Don't link bluetoothd to udev unconditionally, but only when it is > needed. For now bluetoothd needs to be linked to udev only when the > sixaxis plugin is enabled. Any comment on this change? If it looks OK I'll fold it in v5. Thanks, Antonio > --- > Makefile.am | 6 +++++- > 1 files changed, 5 insertions(+), 1 deletions(-) > > diff --git a/Makefile.am b/Makefile.am > index dbe0170..219ca0f 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -288,7 +288,11 @@ src_bluetoothd_SOURCES = $(gdbus_sources) $(builtin_sources) \ > src/event.h src/event.c \ > src/oob.h src/oob.c src/eir.h src/eir.c > src_bluetoothd_LDADD = lib/libbluetooth.la @GLIB_LIBS@ @DBUS_LIBS@ \ > - @CAPNG_LIBS@ @UDEV_LIBS@ -ldl -lrt > + @CAPNG_LIBS@ -ldl -lrt > +if SIXAXISPLUGIN > +src_bluetoothd_LDADD += @UDEV_LIBS@ > +endif > + > src_bluetoothd_LDFLAGS = -Wl,--export-dynamic \ > -Wl,--version-script=$(srcdir)/src/bluetooth.ver > > -- > 1.7.5.4 > >
Hi Antonio, > > > Don't link bluetoothd to udev unconditionally, but only when it is > > needed. For now bluetoothd needs to be linked to udev only when the > > sixaxis plugin is enabled. > > Any comment on this change? If it looks OK I'll fold it in v5. > > Thanks, > Antonio > > > --- > > Makefile.am | 6 +++++- > > 1 files changed, 5 insertions(+), 1 deletions(-) > > > > diff --git a/Makefile.am b/Makefile.am > > index dbe0170..219ca0f 100644 > > --- a/Makefile.am > > +++ b/Makefile.am > > @@ -288,7 +288,11 @@ src_bluetoothd_SOURCES = $(gdbus_sources) $(builtin_sources) \ > > src/event.h src/event.c \ > > src/oob.h src/oob.c src/eir.h src/eir.c > > src_bluetoothd_LDADD = lib/libbluetooth.la @GLIB_LIBS@ @DBUS_LIBS@ \ > > - @CAPNG_LIBS@ @UDEV_LIBS@ -ldl -lrt > > + @CAPNG_LIBS@ -ldl -lrt > > +if SIXAXISPLUGIN > > +src_bluetoothd_LDADD += @UDEV_LIBS@ > > +endif > > + > > src_bluetoothd_LDFLAGS = -Wl,--export-dynamic \ > > -Wl,--version-script=$(srcdir)/src/bluetooth.ver I rather have the Sixaxis plugin being an external plugin only then. So you are not allowed to make this builtin. Regards Marcel -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 19 Aug 2011 21:08:54 +0200 Antonio Ospite <ospite@studenti.unina.it> wrote: > On Fri, 19 Aug 2011 10:58:02 +0200 > Antonio Ospite <ospite@studenti.unina.it> wrote: > > > On Thu, 18 Aug 2011 16:18:57 -0700 > > Marcel Holtmann <marcel@holtmann.org> wrote: > > > [...] > > > > > > I rather have the Sixaxis plugin being an external plugin only then. So > > > you are not allowed to make this builtin. > > > > > > > Marcel, I guess that an _external_ plugin in BlueZ context is one which > > is not embedded in the bluetoothd binary but rather distributed as a > > shared object, isn't it? Are there examples of such a plugin in bluez > > tree I can copy from? > > > > OK, I think I've got it, it is external-dummy, this is how I did it: > > --- > Makefile.am | 10 ++++++---- > 1 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/Makefile.am b/Makefile.am > index 4bcff4d..ff33bf0 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -230,8 +230,10 @@ builtin_sources += thermometer/main.c \ > endif > > if SIXAXISPLUGIN > -builtin_modules += sixaxis > -builtin_sources += plugins/sixaxis.c > +plugin_LTLIBRARIES += plugins/sixaxis.la > +plugins_sixaxis_la_SOURCES = plugins/sixaxis.c > +plugins_sixaxis_la_LDFLAGS = -module -avoid-version -no-undefined @UDEV_LIBS@ > +plugins_sixaxis_la_CFLAGS = -fvisibility=hidden @DBUS_CFLAGS@ @GLIB_CFLAGS@ @UDEV_CFLAGS@ > endif > This is not enough, the external plugin can be compiled but I can be used at runtime because it is using functions from src/storage.c and others, which seem to be embedded inside bluetoothd only, so I get: bluetoothd[23459]: Can't load plugin \ /home/ao2/Proj/PS3/PS3_sixaxis/bluez/plugins/.libs/playstation-peripheral.so: \ /home/ao2/Proj/PS3/PS3_sixaxis/bluez/plugins/.libs/playstation-peripheral.so: \ undefined symbol: write_device_name I could link the needed object files like storage.o and company into the plugin binary, but definitely this doesn't look OK. Should a shared library provide those "utility" functions to bluetoothd _and_ external plugins? This plugin is the first non-trivial external plugin I can see in the tree, so maybe this problem has never showed up before. Thanks, Antonio
Hi Antonio, On 16:14 Thu 25 Aug, Antonio Ospite wrote: > On Fri, 19 Aug 2011 21:08:54 +0200 > Antonio Ospite <ospite@studenti.unina.it> wrote: > > > On Fri, 19 Aug 2011 10:58:02 +0200 > > Antonio Ospite <ospite@studenti.unina.it> wrote: > > > > > On Thu, 18 Aug 2011 16:18:57 -0700 > > > Marcel Holtmann <marcel@holtmann.org> wrote: > > > > > [...] > > > > > > > > I rather have the Sixaxis plugin being an external plugin only then. So > > > > you are not allowed to make this builtin. > > > > > > > > > > Marcel, I guess that an _external_ plugin in BlueZ context is one which > > > is not embedded in the bluetoothd binary but rather distributed as a > > > shared object, isn't it? Are there examples of such a plugin in bluez > > > tree I can copy from? > > > > > > > OK, I think I've got it, it is external-dummy, this is how I did it: > > > > --- > > Makefile.am | 10 ++++++---- > > 1 files changed, 6 insertions(+), 4 deletions(-) > > > > diff --git a/Makefile.am b/Makefile.am > > index 4bcff4d..ff33bf0 100644 > > --- a/Makefile.am > > +++ b/Makefile.am > > @@ -230,8 +230,10 @@ builtin_sources += thermometer/main.c \ > > endif > > > > if SIXAXISPLUGIN > > -builtin_modules += sixaxis > > -builtin_sources += plugins/sixaxis.c > > +plugin_LTLIBRARIES += plugins/sixaxis.la > > +plugins_sixaxis_la_SOURCES = plugins/sixaxis.c > > +plugins_sixaxis_la_LDFLAGS = -module -avoid-version -no-undefined @UDEV_LIBS@ > > +plugins_sixaxis_la_CFLAGS = -fvisibility=hidden @DBUS_CFLAGS@ @GLIB_CFLAGS@ @UDEV_CFLAGS@ > > endif > > > > This is not enough, the external plugin can be compiled but I can be used > at runtime because it is using functions from src/storage.c and others, > which seem to be embedded inside bluetoothd only, so I get: > > bluetoothd[23459]: Can't load plugin \ > /home/ao2/Proj/PS3/PS3_sixaxis/bluez/plugins/.libs/playstation-peripheral.so: \ > /home/ao2/Proj/PS3/PS3_sixaxis/bluez/plugins/.libs/playstation-peripheral.so: \ > undefined symbol: write_device_name > > I could link the needed object files like storage.o and company into the > plugin binary, but definitely this doesn't look OK. Should a shared library > provide those "utility" functions to bluetoothd _and_ external plugins? > > This plugin is the first non-trivial external plugin I can see in the tree, > so maybe this problem has never showed up before. The problem is the visibility of the symbols, src/bluetooth.ver defines which symbols should[1] be visible to plugins. Every plugin should only depend on those methods from the "core". If we want to keep the sixaxis plugin external we need to make the symbols it need visible, but I wonder if that is right solution. [1] Seems that with builtin plugins that visibility enforcement doesn't work, right? > > Thanks, > Antonio > > -- > Antonio Ospite > http://ao2.it > > PGP public key ID: 0x4553B001 > > A: Because it messes up the order in which people normally read text. > See http://en.wikipedia.org/wiki/Posting_style > Q: Why is top-posting such a bad thing? Cheers,
diff --git a/Makefile.am b/Makefile.am index dbe0170..219ca0f 100644 --- a/Makefile.am +++ b/Makefile.am @@ -288,7 +288,11 @@ src_bluetoothd_SOURCES = $(gdbus_sources) $(builtin_sources) \ src/event.h src/event.c \ src/oob.h src/oob.c src/eir.h src/eir.c src_bluetoothd_LDADD = lib/libbluetooth.la @GLIB_LIBS@ @DBUS_LIBS@ \ - @CAPNG_LIBS@ @UDEV_LIBS@ -ldl -lrt + @CAPNG_LIBS@ -ldl -lrt +if SIXAXISPLUGIN +src_bluetoothd_LDADD += @UDEV_LIBS@ +endif + src_bluetoothd_LDFLAGS = -Wl,--export-dynamic \ -Wl,--version-script=$(srcdir)/src/bluetooth.ver