[PATCH] weston: catch missing commandline values

[Bryce Harrington]

On Thu, May 12, 2016 at 02:47:15PM -0700, Florian Hänel wrote:
> took me long enough that --tty 2 is wrong snce it does not give an error.
> I did not use weston_log since not everyone using the parser has that.
> 
> Maybe we should also check for EINVAL from strtol.
> 
> --Florian

> >From 2730944866ddf3d00db852defedeaf7467c1259a Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Florian=20H=C3=A4nel?= <florian.haenel at lge.com>
> Date: Thu, 12 May 2016 14:40:44 -0700
> Subject: [PATCH] Catch missing commandline values
> 
> Exit if commandline options are missing values.
> 
> Tested with:
> weston --tty
> weston --tty 2
> weston --tty=
> ---
>  shared/option-parser.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/shared/option-parser.c b/shared/option-parser.c
> index 7061268..7222fa8 100644
> --- a/shared/option-parser.c
> +++ b/shared/option-parser.c
> @@ -34,6 +34,10 @@ static int
>  handle_option(const struct weston_option *option, char *value)
>  {
>  	char* p;
> +	if (strlen(value) == 0) {
> +		fprintf(stderr, "\nERROR: expected value after --%s=\n", option->name);
> +		exit(EXIT_FAILURE);
> +	}

It looks like this routine returns false if it couldn't parse things,
however the callers to parse_options don't really do error handling, so
exiting probably is the right course of action.  In any case I'm not
sure we'd be expected to recover and carry on when the command line was
bad.  So almost a R-b, however spotted one error:

>  	switch (option->type) {
>  	case WESTON_OPTION_INTEGER:
> @@ -46,7 +50,8 @@ handle_option(const struct weston_option *option, char *value)
>  		* (char **) option->data = strdup(value);
>  		return 1;
>  	default:
> -		assert(0);
> +		fprintf(stderr, "\nERROR: internal commandline parsing error.\n", option->name);

The last arg isn't handled here; needs a %s in the msg?

> +		exit(EXIT_FAILURE);
>  	}
>  }
>  
> @@ -71,6 +76,9 @@ long_option(const struct weston_option *options, int count, char *arg)
>  			}
>  		} else if (arg[len+2] == '=') {
>  			return handle_option(options + k, arg + len + 3);
> +		} else {
> +			fprintf(stderr, "\nERROR: expected syntax: --%s=<value>\n", options[k].name);
> +			exit(EXIT_FAILURE);

If you fix the above and resubmit, I'd be ok landing this for 1.11 as
it's straightforward enough.

Bryce