Re: [PATCH v2] shared/shell: Fix --init-script commandline option

[Juerg Haefliger <juerg.haefliger@canonical.com>]

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

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]