pbap: use the public DBus connection
v2 cover pbap: use the public DBus connection
DBus requests are received by the public DBus connection, because it registers the relevant bus name. My previous patches told PBAP to listen on a private connection, but DBus doesn't let connections share bus names, so it needs to listen on the public connection instead.
This adds a little more complexity to the logind callbacks, to avoid unregistering profiles during shutdown. It also removes an API I previously added, which is no longer used and would only encourage the next person to make the same mistake.
While reviewing this change, it became obvious seat-detection should be merged into the normal driver interface instead. That change is likely to add time and could create its own bugs, so this series fixes the simple bug while we discuss the deeper issue.
--
Frédéric and Pauli - please let me know if you'd like to be CCed on the architecture discussion. Otherwise, I'll assume you're not interested in that part of the problem.
V1 -> V2 Move instances of "(void)foo" below "int bar"
(fixes a "mixed declarations and code" error)
Andrew Sayers (3):
obexd: Pass at_(un)register value to logind callbacks
pbap: use the public DBus connection
Revert "obexd: Support creating private system/session bus connections"
obexd/client/pbap.c | 28 ++++++++++++++++++++++++---- obexd/plugins/bluetooth.c | 6 ++++-- obexd/src/logind.c | 14 +++++++------- obexd/src/logind.h | 8 ++++---- obexd/src/main.c | 8 -------- obexd/src/obexd.h | 2 -- 6 files changed, 39 insertions(+), 27 deletions(-)
PATCH v2 1/3 obexd: Pass at_(un)register value to logind callbacks
Logind (un)registers callbacks that it calls when the user's state changes. Callbacks may also be called during (un)registration.
Clients may need to handle those initial/final calls specially.
Pass an argument indicating whether this is being called during (un)registration, and modify existing callbacks to ignore that argument.
--- obexd/client/pbap.c | 6 ++++-- obexd/plugins/bluetooth.c | 6 ++++-- obexd/src/logind.c | 14 +++++++------- obexd/src/logind.h | 8 ++++---- 4 files changed, 19 insertions(+), 15 deletions(-)
obexd/client/pbap.c +4 -2
@@ -1455,9 +1455,10 @@ static struct obc_driver pbap = { .remove = pbap_remove }; -static int pbap_init_cb (void ) +static int pbap_init_cb (gboolean at_register) {int err; + (void )at_register;DBG ("" );
@@ -1482,8 +1483,9 @@ static int pbap_init_cb(void)return 0 ; } -static void pbap_exit_cb (void ) +static void pbap_exit_cb (gboolean at_unregister) { + (void )at_unregister;DBG ("" );g_dbus_remove_watch (system_conn, listener_id);
obexd/plugins/bluetooth.c +4 -2
@@ -427,8 +427,9 @@ static const struct obex_transport_driver driver = {static unsigned int listener_id =0 ; -static int bluetooth_init_cb (void ) +static int bluetooth_init_cb (gboolean at_register) { + (void )at_register; connection =g_dbus_setup_private (DBUS_BUS_SYSTEM ,NULL ,NULL );if (connection ==NULL )return -EPERM ;
@@ -439,9 +440,10 @@ static int bluetooth_init_cb(void)return obex_transport_driver_register (&driver); } -static void bluetooth_exit_cb (void ) +static void bluetooth_exit_cb (gboolean at_unregister) {GSList *l; + (void )at_unregister;g_dbus_remove_watch (connection, listener_id);
obexd/src/logind.c +7 -7
@@ -43,13 +43,13 @@ static void call_init_cb(gpointer data, gpointer user_data) {int res; - res = ((struct callback_pair *)data)->init_cb (); + res = ((struct callback_pair *)data)->init_cb (FALSE);if (res) *(int *)user_data = res; }static void call_exit_cb (gpointer data,gpointer user_data) { - ((struct callback_pair *)data)->exit_cb (); + ((struct callback_pair *)data)->exit_cb (FALSE); }static int update (void )
@@ -229,7 +229,7 @@ int logind_register(logind_init_cb init_cb, logind_exit_cb exit_cb)struct callback_pair *cbs;if (!monitoring_enabled) -return init_cb (); +return init_cb (TRUE);if (callbacks ==NULL ) {int res;
@@ -237,23 +237,23 @@ int logind_register(logind_init_cb init_cb, logind_exit_cb exit_cb)if (res) {error ("logind_init(): %s - login detection disabled" ,strerror (-res)); -return init_cb (); +return init_cb (TRUE); } } cbs = g_new(struct callback_pair, 1); cbs->init_cb = init_cb; cbs->exit_cb = exit_cb; callbacks =g_slist_prepend (callbacks, cbs); -return active ?init_cb () :0 ; +return active ?init_cb (TRUE) :0 ; }void logind_unregister (logind_init_cb init_cb,logind_exit_cb exit_cb) {GSList *cb_node;if (!monitoring_enabled) -return exit_cb (); +return exit_cb (TRUE);if (active) -exit_cb (); +exit_cb (TRUE); cb_node =g_slist_find_custom (callbacks, init_cb, find_cb);if (cb_node !=NULL ) callbacks =g_slist_delete_link (callbacks, cb_node);
obexd/src/logind.h +4 -4
@@ -8,8 +8,8 @@* */ -typedef int (*logind_init_cb )(void ); -typedef void (*logind_exit_cb )(void ); +typedef int (*logind_init_cb )(gboolean at_register); +typedef void (*logind_exit_cb )(gboolean at_unregister);#ifdef SYSTEMD
@@ -22,12 +22,12 @@ int logind_set(gboolean enabled);static inline int logind_register (logind_init_cb init_cb,logind_exit_cb exit_cb) { -return init_cb (); +return init_cb (TRUE); }static inline void logind_unregister (logind_init_cb init_cb,logind_exit_cb exit_cb) { -return exit_cb (); +return exit_cb (TRUE); }static inline int logind_set (gboolean enabled) {
Signature · 1 line
2.49.0
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=970848
---Test result---
Test Summary:
CheckPatch PENDING 0.34 seconds GitLint PENDING 0.35 seconds BuildEll PASS 19.98 seconds BluezMake PASS 2634.06 seconds MakeCheck PASS 20.60 seconds MakeDistcheck PASS 198.09 seconds CheckValgrind PASS 275.31 seconds CheckSmatch PASS 306.04 seconds bluezmakeextell PASS 129.10 seconds IncrementalBuild PENDING 0.37 seconds ScanBuild PASS 905.63 seconds
Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:
##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series Output:
---
Regards,
Linux Bluetooth
PATCH v2 2/3 pbap: use the public DBus connection
In reply to Andrew SayersPBAP must not use its own private DBus connection, as it needs to receive messages sent to the bus name of the public connection.
PBAP must not unregister itself when the program is exiting, as it might cause a long delay. Unregistering at exit is redundant anyway, as the service will be implicitly unregistered when the connection is closed.
But PBAP must unregister itself when the user becomes inactive mid-session, so systems with multiple users logged in at once can share bluetooth.
Use the public DBus connection instead of a private one, and explicitly unregister the profile if appropriate.
Thanks Pauli Virtanen for pointing out the exit issue:
https://lore.kernel.org/linux-bluetooth/[email protected]/
--- obexd/client/pbap.c | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-)
obexd/client/pbap.c +21 -3
@@ -1462,7 +1462,7 @@ static int pbap_init_cb(gboolean at_register)DBG ("" ); - conn =obex_setup_dbus_connection_private (NULL ,NULL ); + conn =obex_get_dbus_connection ();if (!conn)return -EIO ;
@@ -1485,9 +1485,28 @@ static int pbap_init_cb(gboolean at_register)static void pbap_exit_cb (gboolean at_unregister) { - (void )at_unregister; +DBusMessage *msg; +DBusMessageIter iter; +char *uuid =PBAP_CLIENT_UUID ; +DBG ("" ); +if (!at_unregister) { + client_path =g_strconcat ("/org/bluez/obex/" , uuid,NULL ); +g_strdelimit (client_path,"-" ,'_' ); + + msg =dbus_message_new_method_call ("org.bluez" ,"/org/bluez" , +"org.bluez.ProfileManager1" , +"UnregisterProfile" ); + +dbus_message_iter_init_append (msg, &iter); + +dbus_message_iter_append_basic (&iter,DBUS_TYPE_OBJECT_PATH , + &client_path); + +g_dbus_send_message (system_conn, msg); + } +g_dbus_remove_watch (system_conn, listener_id);unregister_profile ();
@@ -1499,7 +1518,6 @@ static void pbap_exit_cb(gboolean at_unregister) }if (conn) { -dbus_connection_close (conn);dbus_connection_unref (conn); conn =NULL ; }
Signature · 1 line
2.49.0
PATCH v2 3/3 Revert "obexd: Support creating private system/session bus connections"
In reply to Andrew SayersThis reverts commit 237d818ef294e22be87fba69b3cdd79c75c201e7, which encouraged the use of private DBus connections.
Doing so caused a bug, so it's better to remove the temptation for now and re-add it if a legitimate use case is ever found.
--- obexd/src/main.c | 8 -------- obexd/src/obexd.h | 2 -- 2 files changed, 10 deletions(-)
obexd/src/main.c +0 -8
@@ -254,14 +254,6 @@ DBusConnection *obex_setup_dbus_connection(const char *name,return connection; } -DBusConnection *obex_setup_dbus_connection_private (const char *name, -DBusError *error) -{ -return g_dbus_setup_private (option_system_bus ? -DBUS_BUS_SYSTEM :DBUS_BUS_SESSION , - name, error); -} -int main (int argc,char *argv[]) {GOptionContext *context;
obexd/src/obexd.h +0 -2
@@ -33,5 +33,3 @@ const char *obex_option_capability(void);DBusConnection *obex_get_dbus_connection (void );DBusConnection *obex_setup_dbus_connection (const char *name,DBusError *error); -DBusConnection *obex_setup_dbus_connection_private (const char *name, -DBusError *error);
Signature · 1 line
2.49.0
This series was applied to bluetooth/bluez.git (master) by Luiz Augusto von Dentz <[email protected]>:
Hello:
This series was applied to bluetooth/bluez.git (master) by Luiz Augusto von Dentz <[email protected]>:
Quoted 11 lines · On Wed, 11 Jun 2025 14:06:12 +0100 you wrote:
DBus requests are received by the public DBus connection, because it registers the relevant bus name. My previous patches told PBAP to listen on a private connection, but DBus doesn't let connections share bus names, so it needs to listen on the public connection instead.
This adds a little more complexity to the logind callbacks, to avoid unregistering profiles during shutdown. It also removes an API I previously added, which is no longer used and would only encourage the next person to make the same mistake.
[...]
Here is the summary with links:
- [BlueZ,v2,1/3] obexd: Pass at_(un)register value to logind callbacks
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=883c1c11c644 - [BlueZ,v2,2/3] pbap: use the public DBus connection https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=df0036d9e41f - [BlueZ,v2,3/3] Revert "obexd: Support creating private system/session bus connections" https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=aea0f46f173f
You are awesome, thank you!
Signature · 2 lines
Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html