[BlueZ 0/3] Fix bluetoothctl hanging if daemon isn't running

[BlueZ 0/3] Fix bluetoothctl hanging if daemon isn't running

[Bastien Nocera]

Our QE folks have run into this problem multiple times. The problem seems
to have existed since at least 5.56.

Bastien Nocera (3):
  shared/shell: Add bt_shell_get_timeout()
  client: Respect --timeout when bluetoothd isn't running
  shared/shell: Exit after printing the help

 client/main.c      | 21 +++++++++++++++++++--
 src/shared/shell.c |  9 ++++++---
 src/shared/shell.h |  2 ++
 3 files changed, 27 insertions(+), 5 deletions(-)

-- 
2.46.0

[BlueZ 1/3] shared/shell: Add bt_shell_get_timeout()

[Bastien Nocera]

Make it possible to get the value of the general --timeout option.
---
 src/shared/shell.c | 5 +++++
 src/shared/shell.h | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/src/shared/shell.c b/src/shared/shell.c
index 246de209e41c..c31487190d0f 100644
--- a/src/shared/shell.c
+++ b/src/shared/shell.c
@@ -1670,3 +1670,8 @@ void *bt_shell_get_env(const char *name)
 
 	return env->value;
 }
+
+int bt_shell_get_timeout(void)
+{
+	return data.timeout;
+}
diff --git a/src/shared/shell.h b/src/shared/shell.h
index a9a635bda959..b03250cac80f 100644
--- a/src/shared/shell.h
+++ b/src/shared/shell.h
@@ -85,4 +85,6 @@ bool bt_shell_detach(void);
 void bt_shell_set_env(const char *name, void *value);
 void *bt_shell_get_env(const char *name);
 
+int bt_shell_get_timeout(void);
+
 void bt_shell_cleanup(void);
-- 
2.46.0

RE: Fix bluetoothctl hanging if daemon isn't running

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 2500 bytes --]

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=889687

---Test result---

Test Summary:
CheckPatch                    FAIL      1.44 seconds
GitLint                       PASS      0.89 seconds
BuildEll                      PASS      25.63 seconds
BluezMake                     PASS      1740.30 seconds
MakeCheck                     PASS      12.91 seconds
MakeDistcheck                 PASS      179.84 seconds
CheckValgrind                 PASS      257.61 seconds
CheckSmatch                   WARNING   360.88 seconds
bluezmakeextell               PASS      121.02 seconds
IncrementalBuild              PASS      4687.59 seconds
ScanBuild                     PASS      1026.96 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[BlueZ,2/3] client: Respect --timeout when bluetoothd isn't running
WARNING:LONG_LINE: line length of 83 exceeds 80 columns
#105: FILE: client/main.c:3237:
+		timeout_id = timeout_add(timeout * 1000, timeout_quit, NULL, NULL);

/github/workspace/src/src/13801792.patch total: 0 errors, 1 warnings, 47 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/src/13801792.patch has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
src/shared/shell.c: note: in included file (through /usr/include/readline/readline.h):src/shared/shell.c: note: in included file (through /usr/include/readline/readline.h):src/shared/shell.c: note: in included file (through /usr/include/readline/readline.h):src/shared/shell.c: note: in included file (through /usr/include/readline/readline.h):src/shared/shell.c: note: in included file (through /usr/include/readline/readline.h):src/shared/shell.c: note: in included file (through /usr/include/readline/readline.h):


---
Regards,
Linux Bluetooth

[BlueZ 2/3] client: Respect --timeout when bluetoothd isn't running

[Bastien Nocera]

Exit after <timeout> seconds if bluetoothd isn't available. This
functionality is useful for non-interactive uses of bluetoothctl.
---
 client/main.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/client/main.c b/client/main.c
index a96a4263849d..8791466e49ff 100644
--- a/client/main.c
+++ b/client/main.c
@@ -22,7 +22,9 @@
 
 #include <glib.h>
 
+#include "src/shared/mainloop.h"
 #include "src/shared/shell.h"
+#include "src/shared/timeout.h"
 #include "src/shared/util.h"
 #include "src/shared/ad.h"
 #include "gdbus/gdbus.h"
@@ -3173,13 +3175,25 @@ static const struct bt_shell_opt opt = {
 
 static void client_ready(GDBusClient *client, void *user_data)
 {
+	unsigned int *timeout_id = user_data;
+
+	if (*timeout_id > 0)
+		timeout_remove(*timeout_id);
 	setup_standard_input();
 }
 
+static bool timeout_quit(void *user_data)
+{
+	mainloop_exit_failure();
+	return true;
+}
+
 int main(int argc, char *argv[])
 {
 	GDBusClient *client;
 	int status;
+	int timeout;
+	unsigned int timeout_id;
 
 	bt_shell_init(argc, argv, &opt);
 	bt_shell_set_menu(&main_menu);
@@ -3217,8 +3231,11 @@ int main(int argc, char *argv[])
 	g_dbus_client_set_proxy_handlers(client, proxy_added, proxy_removed,
 							property_changed, NULL);
 
-	g_dbus_client_set_ready_watch(client, client_ready, NULL);
-
+	timeout = bt_shell_get_timeout();
+	timeout_id = 0;
+	if (timeout > 0)
+		timeout_id = timeout_add(timeout * 1000, timeout_quit, NULL, NULL);
+	g_dbus_client_set_ready_watch(client, client_ready, &timeout_id);
 	status = bt_shell_run();
 
 	admin_remove_submenu();
-- 
2.46.0

[BlueZ 3/3] shared/shell: Exit after printing the help

[Bastien Nocera]

Exit after handling --help, so as to avoid the daemon waiting to
communicate with a D-Bus service that might not be running.
---
 src/shared/shell.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/shared/shell.c b/src/shared/shell.c
index c31487190d0f..f7e8b3300373 100644
--- a/src/shared/shell.c
+++ b/src/shared/shell.c
@@ -1324,9 +1324,7 @@ void bt_shell_init(int argc, char **argv, const struct bt_shell_opt *opt)
 			return;
 		case 'h':
 			usage(argc, argv, opt);
-			data.argc = 1;
-			data.argv = &cmplt;
-			data.mode = 1;
+			exit(EXIT_SUCCESS);
 			goto done;
 		case 's':
 			if (optarg && data.init_fd < 0) {
-- 
2.46.0

Re: [BlueZ 3/3] shared/shell: Exit after printing the help

[Luiz Augusto von Dentz]

Hi Bastien,

On Thu, Sep 12, 2024 at 5:29 AM Bastien Nocera <hadess@hadess.net> wrote:
>
> Exit after handling --help, so as to avoid the daemon waiting to
> communicate with a D-Bus service that might not be running.
> ---
>  src/shared/shell.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/src/shared/shell.c b/src/shared/shell.c
> index c31487190d0f..f7e8b3300373 100644
> --- a/src/shared/shell.c
> +++ b/src/shared/shell.c
> @@ -1324,9 +1324,7 @@ void bt_shell_init(int argc, char **argv, const struct bt_shell_opt *opt)
>                         return;
>                 case 'h':
>                         usage(argc, argv, opt);
> -                       data.argc = 1;
> -                       data.argv = &cmplt;
> -                       data.mode = 1;
> +                       exit(EXIT_SUCCESS);

This would undo shared/shell: Print commands when --help option is
given, so you might want to add print_cmds call.

>                         goto done;
>                 case 's':
>                         if (optarg && data.init_fd < 0) {
> --
> 2.46.0
>
>


-- 
Luiz Augusto von Dentz

Re: [BlueZ 3/3] shared/shell: Exit after printing the help

[Bastien Nocera]

On Thu, 2024-09-12 at 11:11 -0400, Luiz Augusto von Dentz wrote:
> Hi Bastien,
> 
> On Thu, Sep 12, 2024 at 5:29 AM Bastien Nocera <hadess@hadess.net>
> wrote:
> > 
> > Exit after handling --help, so as to avoid the daemon waiting to
> > communicate with a D-Bus service that might not be running.
> > ---
> >  src/shared/shell.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/src/shared/shell.c b/src/shared/shell.c
> > index c31487190d0f..f7e8b3300373 100644
> > --- a/src/shared/shell.c
> > +++ b/src/shared/shell.c
> > @@ -1324,9 +1324,7 @@ void bt_shell_init(int argc, char **argv,
> > const struct bt_shell_opt *opt)
> >                         return;
> >                 case 'h':
> >                         usage(argc, argv, opt);
> > -                       data.argc = 1;
> > -                       data.argv = &cmplt;
> > -                       data.mode = 1;
> > +                       exit(EXIT_SUCCESS);
> 
> This would undo shared/shell: Print commands when --help option is
> given, so you might want to add print_cmds call.

I'll need to come back to this.

bt_shell_init() is called before the submenus are added, so I can't
call print_cmds() at that point, I'll need to postpone the exit until
after the menus are registered and their help printed.

> 
> >                         goto done;
> >                 case 's':
> >                         if (optarg && data.init_fd < 0) {
> > --
> > 2.46.0
> > 
> > 
> 
> 

Re: [BlueZ 3/3] shared/shell: Exit after printing the help

[Bastien Nocera]

On Thu, 2024-09-12 at 11:11 -0400, Luiz Augusto von Dentz wrote:
> Hi Bastien,
> 
> On Thu, Sep 12, 2024 at 5:29 AM Bastien Nocera <hadess@hadess.net>
> wrote:
> > 
> > Exit after handling --help, so as to avoid the daemon waiting to
> > communicate with a D-Bus service that might not be running.
> > ---
> >  src/shared/shell.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/src/shared/shell.c b/src/shared/shell.c
> > index c31487190d0f..f7e8b3300373 100644
> > --- a/src/shared/shell.c
> > +++ b/src/shared/shell.c
> > @@ -1324,9 +1324,7 @@ void bt_shell_init(int argc, char **argv,
> > const struct bt_shell_opt *opt)
> >                         return;
> >                 case 'h':
> >                         usage(argc, argv, opt);
> > -                       data.argc = 1;
> > -                       data.argv = &cmplt;
> > -                       data.mode = 1;
> > +                       exit(EXIT_SUCCESS);
> 
> This would undo shared/shell: Print commands when --help option is
> given, so you might want to add print_cmds call.

That wasn't enough, because the menu info isn't populated at this
point, so we'd only get the top-level help. It was quite a bit more
involved, but I hope I got all variants covered in the v2 patchset.

Cheers

> 
> >                         goto done;
> >                 case 's':
> >                         if (optarg && data.init_fd < 0) {
> > --
> > 2.46.0
> > 
> > 
> 
> 

Re: [BlueZ 0/3] Fix bluetoothctl hanging if daemon isn't running

[patchwork-bot+bluetooth]

Hello:

This series was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Thu, 12 Sep 2024 11:20:09 +0200 you wrote:
> Our QE folks have run into this problem multiple times. The problem seems
> to have existed since at least 5.56.
> 
> Bastien Nocera (3):
>   shared/shell: Add bt_shell_get_timeout()
>   client: Respect --timeout when bluetoothd isn't running
>   shared/shell: Exit after printing the help
> 
> [...]

Here is the summary with links:
  - [BlueZ,1/3] shared/shell: Add bt_shell_get_timeout()
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=1428d8461e92
  - [BlueZ,2/3] client: Respect --timeout when bluetoothd isn't running
    (no matching commit)
  - [BlueZ,3/3] shared/shell: Exit after printing the help
    (no matching commit)

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html