On Sat, 13 Feb 2016 23:56:38 +0100 Benoit Gschwind <[email protected]> wrote:
> Hello Bryce, > > It seems the corner case '-f42xxx 2938475' doesn't work as expected with > 'f' short option as integer: > > 1. one dash then call short_option > 2. in short_option will check arg[2] and call handle_option > 3. in handle_option will call strtol, where *value is '4' and *p is 'x' > thus *value && !*p is 0 > 4. return from handle_option with 0 > 5. return from short_option with 0 > 6. call short_option_with_arg with arg = "-f42xxx" and param = "2938475" > 7. pass through all test and call handle_option > 8. give 2938475 to the option > > The expected behavior is an error. > > Should I use Reviewed-by ? No, you only give Reviewed-by when you think everything is good. You are pointing out a problem instead. Looks like this patch landed. Isn't there something to fix? I think we might have tests for the option parser. Would be good to add this particular corner-case as a test there. If we don't have tests, would be nice to have some. Thanks, pq > Le 12/02/2016 00:25, Bryce Harrington a écrit : > > weston allows both short and long style options to take arguments. In > > the case of short options, allow an optional space between the option > > name and value. E.g., previously you could launch weston this way: > > > > weston -i2 -cmyconfig.ini > > > > now you can also launch it like this: > > > > weston -i 2 -c myconfig.ini > > > > Signed-off-by: Bryce Harrington <[email protected]> > > --- > > shared/option-parser.c | 41 ++++++++++++++++++++++++++++++++++++++--- > > 1 file changed, 38 insertions(+), 3 deletions(-) > > > > diff --git a/shared/option-parser.c b/shared/option-parser.c > > index f1cc529..d5fee8e 100644 > > --- a/shared/option-parser.c > > +++ b/shared/option-parser.c > > @@ -98,14 +98,37 @@ short_option(const struct weston_option *options, int > > count, char *arg) > > > > return 1; > > } > > - } else { > > + } else if (arg[2]) { > > return handle_option(options + k, arg + 2); > > + } else { > > + return 0; > > } > > } > > > > return 0; > > } > > > > +static int > > +short_option_with_arg(const struct weston_option *options, int count, char > > *arg, char *param) > > +{ > > + int k; > > + > > + if (!arg[1]) > > + return 0; > > + > > + for (k = 0; k < count; k++) { > > + if (options[k].short_name != arg[1]) > > + continue; > > + > > + if (options[k].type == WESTON_OPTION_BOOLEAN) > > + continue; > > + > > + return handle_option(options + k, param); > > + } > > + > > + return 0; > > +} > > + > > int > > parse_options(const struct weston_option *options, > > int count, int *argc, char *argv[]) > > @@ -115,10 +138,22 @@ parse_options(const struct weston_option *options, > > for (i = 1, j = 1; i < *argc; i++) { > > if (argv[i][0] == '-') { > > if (argv[i][1] == '-') { > > + /* Long option, e.g. --foo or --foo=bar */ > > if (long_option(options, count, argv[i])) > > continue; > > - } else if (short_option(options, count, argv[i])) > > - continue; > > + > > + } else { > > + /* Short option, e.g -f or -f42 */ > > + if (short_option(options, count, argv[i])) > > + continue; > > + > > + /* ...also handle -f 42 */ > > + if (i+1 < *argc && > > + short_option_with_arg(options, count, > > argv[i], argv[i+1])) { > > + i++; > > + continue; > > + } > > + } > > } > > argv[j++] = argv[i]; > > } > > > _______________________________________________ > wayland-devel mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/wayland-devel
pgpiEqoUtjzWz.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
