mbox series

[BlueZ,v2,0/7] Fix bluetoothctl --help hanging if daemon isn't running

Message ID 20241022120051.123888-1-hadess@hadess.net
Headers show
Series Fix bluetoothctl --help hanging if daemon isn't running | expand

Message

Bastien Nocera Oct. 22, 2024, 11:58 a.m. UTC
v2, this one can print all the submenus without requiring bluetoothd
to be running.

Bastien Nocera (7):
  configure.ac: Update requirement to glib 2.34
  client: Use g_clear_pointer() to clean up menus
  client: Split installing submenu and doing I/O
  client: Install submenus before contacting bluez daemon
  shell: Document "mode" variable
  shared/shell: Add function to handle early help calls
  client: Fix --help hanging if bluetoothd is not running

 acinclude.m4       |  4 ++--
 client/admin.c     |  8 +++++---
 client/admin.h     |  1 +
 client/assistant.c |  8 +++++---
 client/assistant.h |  1 +
 client/main.c      | 14 ++++++++++----
 client/mgmt.c      | 11 +++++++----
 client/mgmt.h      |  3 ++-
 client/player.c    |  7 +++++--
 client/player.h    |  1 +
 configure.ac       |  2 +-
 src/shared/shell.c | 12 +++++++++++-
 src/shared/shell.h |  2 ++
 tools/btmgmt.c     |  3 ++-
 14 files changed, 55 insertions(+), 22 deletions(-)

Comments

Bastien Nocera Oct. 22, 2024, 3:55 p.m. UTC | #1
On Tue, 2024-10-22 at 10:59 -0400, Luiz Augusto von Dentz wrote:
> Hi Bastien,
> 
> On Tue, Oct 22, 2024 at 8:01 AM Bastien Nocera <hadess@hadess.net>
> wrote:
> > 
> > This would avoid warnings should the client be NULL.
> > ---
> >  client/admin.c     | 5 ++---
> >  client/assistant.c | 5 ++---
> >  client/player.c    | 4 ++--
> >  3 files changed, 6 insertions(+), 8 deletions(-)
> > 
> > diff --git a/client/admin.c b/client/admin.c
> > index cd9af6f955da..9d48867bc1d7 100644
> > --- a/client/admin.c
> > +++ b/client/admin.c
> > @@ -191,7 +191,7 @@ static void proxy_removed(GDBusProxy *proxy,
> > void *user_data)
> >                 admin_policy_status_removed(proxy);
> >  }
> > 
> > -static GDBusClient *client;
> > +static GDBusClient *client = NULL;
> > 
> >  static void disconnect_handler(DBusConnection *connection, void
> > *user_data)
> >  {
> > @@ -215,6 +215,5 @@ void admin_add_submenu(void)
> > 
> >  void admin_remove_submenu(void)
> >  {
> > -       g_dbus_client_unref(client);
> > -       client = NULL;
> > +       g_clear_pointer(&client, g_dbus_client_unref);
> 
> Don't think it is worth bumping the glib dependency just to add a
> NULL
> pointer check, in fact g_dbus_client_unref should be safe to be
> called
> with NULL pointers so I wonder if this is a false positive.

You're right, it's not the GLib gdbus. Feel free to skip this and the
preceding patch then. It should rebase cleanly, but I can send a v4 if
needed.

> 
> >  }
> > diff --git a/client/assistant.c b/client/assistant.c
> > index 16e94664a5c3..94052e26fd59 100644
> > --- a/client/assistant.c
> > +++ b/client/assistant.c
> > @@ -390,7 +390,7 @@ static const struct bt_shell_menu
> > assistant_menu = {
> >         {} },
> >  };
> > 
> > -static GDBusClient * client;
> > +static GDBusClient * client = NULL;
> > 
> >  void assistant_add_submenu(void)
> >  {
> > @@ -409,7 +409,6 @@ void assistant_add_submenu(void)
> > 
> >  void assistant_remove_submenu(void)
> >  {
> > -       g_dbus_client_unref(client);
> > -       client = NULL;
> > +       g_clear_pointer(&client, g_dbus_client_unref);
> >  }
> > 
> > diff --git a/client/player.c b/client/player.c
> > index 188378175486..dea5922d56db 100644
> > --- a/client/player.c
> > +++ b/client/player.c
> > @@ -5694,7 +5694,7 @@ static const struct bt_shell_menu
> > transport_menu = {
> >         {} },
> >  };
> > 
> > -static GDBusClient *client;
> > +static GDBusClient *client = NULL;
> > 
> >  void player_add_submenu(void)
> >  {
> > @@ -5715,6 +5715,6 @@ void player_add_submenu(void)
> > 
> >  void player_remove_submenu(void)
> >  {
> > -       g_dbus_client_unref(client);
> > +       g_clear_pointer(&client, g_dbus_client_unref);
> >         queue_destroy(ios, transport_free);
> >  }
> > --
> > 2.47.0
> > 
> > 
> 
>