diff mbox series

[BlueZ,v2,2/7] client: Use g_clear_pointer() to clean up menus

Message ID 20241022120051.123888-3-hadess@hadess.net (mailing list archive)
State Superseded
Headers show
Series Fix bluetoothctl --help hanging if daemon isn't running | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success

Commit Message

Bastien Nocera Oct. 22, 2024, 11:58 a.m. UTC 
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(-)

Comments

Luiz Augusto von Dentz Oct. 22, 2024, 2:59 p.m. UTC | #1 
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.

>  }
> 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
>
>
Bastien Nocera Oct. 22, 2024, 3:55 p.m. UTC | #2 
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
> > 
> > 
> 
>
diff mbox series

Patch

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);
 }
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);
 }