| Message ID | 20231030065341.8998-1-juerg.haefliger@canonical.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 303925b28110469ad002ac19ce0eb9c84d6aceb2 |
| Headers | show |
| Series | [v2] shared/shell: Fix --init-script commandline option | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/CheckPatch | success | CheckPatch PASS |
| tedd_an/GitLint | success | Gitlint PASS |
| tedd_an/BuildEll | success | Build ELL PASS |
| tedd_an/BluezMake | success | Bluez Make PASS |
| tedd_an/MakeCheck | success | Bluez Make Check PASS |
| tedd_an/MakeDistcheck | success | Make Distcheck PASS |
| tedd_an/CheckValgrind | success | Check Valgrind PASS |
| tedd_an/CheckSmatch | warning | CheckSparse WARNING 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): |
| tedd_an/bluezmakeextell | success | Make External ELL PASS |
| tedd_an/IncrementalBuild | success | Incremental Build PASS |
| tedd_an/ScanBuild | success | Scan Build PASS |
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=797481 ---Test result--- Test Summary: CheckPatch PASS 0.42 seconds GitLint PASS 0.29 seconds BuildEll PASS 29.61 seconds BluezMake PASS 951.63 seconds MakeCheck PASS 13.17 seconds MakeDistcheck PASS 184.63 seconds CheckValgrind PASS 282.56 seconds CheckSmatch WARNING 379.14 seconds bluezmakeextell PASS 121.60 seconds IncrementalBuild PASS 785.02 seconds ScanBuild PASS 1164.83 seconds Details ############################## 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): --- Regards, Linux Bluetooth
Hi Juerg, On Mon, Oct 30, 2023 at 2:54 AM Juerg Haefliger <juerg.haefliger@canonical.com> wrote: > > The newly added option -i/--init-script introduced a short option > namespace collision with btmgmt's --index, both of which use '-i'. > > As a result, a provided --index is treated as a file name: > > $ sudo btmgmt --index 0 info Perhaps we could remove this --index since btmgmt supports setting the index via select command; it doesn't seem very useful to have 2 different forms of selecting the index. > Unable to open 0: No such file or directory (2) > > Fix this by using '-s' for --init-script. > > Fixes: https://github.com/bluez/bluez/issues/639 > Signed-off-by: Juerg Haefliger <juerg.haefliger@canonical.com> > > --- > v2: > - Replace reference to broken commit with reference to github issue. > --- > src/shared/shell.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/src/shared/shell.c b/src/shared/shell.c > index db79c882ca3a..fbccff5b54d9 100644 > --- a/src/shared/shell.c > +++ b/src/shared/shell.c > @@ -1128,7 +1128,7 @@ static void rl_init(void) > static const struct option main_options[] = { > { "version", no_argument, 0, 'v' }, > { "help", no_argument, 0, 'h' }, > - { "init-script", required_argument, 0, 'i' }, > + { "init-script", required_argument, 0, 's' }, > { "timeout", required_argument, 0, 't' }, > { "monitor", no_argument, 0, 'm' }, > { "zsh-complete", no_argument, 0, 'z' }, > @@ -1169,9 +1169,9 @@ void bt_shell_init(int argc, char **argv, const struct bt_shell_opt *opt) > if (opt) { > memcpy(options + offset, opt->options, > sizeof(struct option) * opt->optno); > - snprintf(optstr, sizeof(optstr), "+mhvi:t:%s", opt->optstr); > + snprintf(optstr, sizeof(optstr), "+mhvs:t:%s", opt->optstr); > } else > - snprintf(optstr, sizeof(optstr), "+mhvi:t:"); > + snprintf(optstr, sizeof(optstr), "+mhvs:t:"); > > data.name = strrchr(argv[0], '/'); > if (!data.name) > @@ -1193,7 +1193,7 @@ void bt_shell_init(int argc, char **argv, const struct bt_shell_opt *opt) > data.argv = &cmplt; > data.mode = 1; > goto done; > - case 'i': > + case 's': > if (optarg) > data.init_fd = open(optarg, O_RDONLY); > if (data.init_fd < 0) > -- > 2.39.2 >
Hi Luiz, > Hi Juerg, > > On Mon, Oct 30, 2023 at 2:54 AM Juerg Haefliger > <juerg.haefliger@canonical.com> wrote: > > > > The newly added option -i/--init-script introduced a short option > > namespace collision with btmgmt's --index, both of which use '-i'. > > > > As a result, a provided --index is treated as a file name: > > > > $ sudo btmgmt --index 0 info > > Perhaps we could remove this --index since btmgmt supports setting the > index via select command; it doesn't seem very useful to have 2 > different forms of selecting the index. That would break a potentially large number of existing scripts, which is bad. --index has been around for a long time and is also supported by other bluez commands. It would require some warning first that it's going away to give people time to transition. I'd rather fix the new option which hasn't been released yet and introduced this regression. ...Juerg > > Unable to open 0: No such file or directory (2) > > > > Fix this by using '-s' for --init-script. > > > > Fixes: https://github.com/bluez/bluez/issues/639 > > Signed-off-by: Juerg Haefliger <juerg.haefliger@canonical.com> > > > > --- > > v2: > > - Replace reference to broken commit with reference to github issue. > > --- > > src/shared/shell.c | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/src/shared/shell.c b/src/shared/shell.c > > index db79c882ca3a..fbccff5b54d9 100644 > > --- a/src/shared/shell.c > > +++ b/src/shared/shell.c > > @@ -1128,7 +1128,7 @@ static void rl_init(void) > > static const struct option main_options[] = { > > { "version", no_argument, 0, 'v' }, > > { "help", no_argument, 0, 'h' }, > > - { "init-script", required_argument, 0, 'i' }, > > + { "init-script", required_argument, 0, 's' }, > > { "timeout", required_argument, 0, 't' }, > > { "monitor", no_argument, 0, 'm' }, > > { "zsh-complete", no_argument, 0, 'z' }, > > @@ -1169,9 +1169,9 @@ void bt_shell_init(int argc, char **argv, const struct bt_shell_opt *opt) > > if (opt) { > > memcpy(options + offset, opt->options, > > sizeof(struct option) * opt->optno); > > - snprintf(optstr, sizeof(optstr), "+mhvi:t:%s", opt->optstr); > > + snprintf(optstr, sizeof(optstr), "+mhvs:t:%s", opt->optstr); > > } else > > - snprintf(optstr, sizeof(optstr), "+mhvi:t:"); > > + snprintf(optstr, sizeof(optstr), "+mhvs:t:"); > > > > data.name = strrchr(argv[0], '/'); > > if (!data.name) > > @@ -1193,7 +1193,7 @@ void bt_shell_init(int argc, char **argv, const struct bt_shell_opt *opt) > > data.argv = &cmplt; > > data.mode = 1; > > goto done; > > - case 'i': > > + case 's': > > if (optarg) > > data.init_fd = open(optarg, O_RDONLY); > > if (data.init_fd < 0) > > -- > > 2.39.2 > > > >
Hello: This patch was applied to bluetooth/bluez.git (master) by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>: On Mon, 30 Oct 2023 07:53:41 +0100 you wrote: > The newly added option -i/--init-script introduced a short option > namespace collision with btmgmt's --index, both of which use '-i'. > > As a result, a provided --index is treated as a file name: > > $ sudo btmgmt --index 0 info > Unable to open 0: No such file or directory (2) > > [...] Here is the summary with links: - [v2] shared/shell: Fix --init-script commandline option https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=303925b28110 You are awesome, thank you!
diff --git a/src/shared/shell.c b/src/shared/shell.c index db79c882ca3a..fbccff5b54d9 100644 --- a/src/shared/shell.c +++ b/src/shared/shell.c @@ -1128,7 +1128,7 @@ static void rl_init(void) static const struct option main_options[] = { { "version", no_argument, 0, 'v' }, { "help", no_argument, 0, 'h' }, - { "init-script", required_argument, 0, 'i' }, + { "init-script", required_argument, 0, 's' }, { "timeout", required_argument, 0, 't' }, { "monitor", no_argument, 0, 'm' }, { "zsh-complete", no_argument, 0, 'z' }, @@ -1169,9 +1169,9 @@ void bt_shell_init(int argc, char **argv, const struct bt_shell_opt *opt) if (opt) { memcpy(options + offset, opt->options, sizeof(struct option) * opt->optno); - snprintf(optstr, sizeof(optstr), "+mhvi:t:%s", opt->optstr); + snprintf(optstr, sizeof(optstr), "+mhvs:t:%s", opt->optstr); } else - snprintf(optstr, sizeof(optstr), "+mhvi:t:"); + snprintf(optstr, sizeof(optstr), "+mhvs:t:"); data.name = strrchr(argv[0], '/'); if (!data.name) @@ -1193,7 +1193,7 @@ void bt_shell_init(int argc, char **argv, const struct bt_shell_opt *opt) data.argv = &cmplt; data.mode = 1; goto done; - case 'i': + case 's': if (optarg) data.init_fd = open(optarg, O_RDONLY); if (data.init_fd < 0)
The newly added option -i/--init-script introduced a short option namespace collision with btmgmt's --index, both of which use '-i'. As a result, a provided --index is treated as a file name: $ sudo btmgmt --index 0 info Unable to open 0: No such file or directory (2) Fix this by using '-s' for --init-script. Fixes: https://github.com/bluez/bluez/issues/639 Signed-off-by: Juerg Haefliger <juerg.haefliger@canonical.com> --- v2: - Replace reference to broken commit with reference to github issue. --- src/shared/shell.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)