[PATCH] option-parser: Handle short double-arg options

[Pekka Paalanen]

On Sat, 13 Feb 2016 23:56:38 +0100
Benoit Gschwind <gschwind at gnu-log.net> 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 <bryce at osg.samsung.com>
> > ---
> >   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
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160414/07cbc912/attachment.sig>