[PATCH v3] parser: be more picky for integer values

[Pekka Paalanen]

On Fri, 16 Mar 2012 15:12:14 -0300
Tiago Vignatti <tiago.vignatti at intel.com> wrote:

> It was silently accepting "-i=3", "-i=3/2", "--idle-time=*3" and similar
> unwanted type of arguments, not changing anything internally; this
> gives the wrong impression for the user. Now it explicitly warns.
> 
> Signed-off-by: Tiago Vignatti <tiago.vignatti at intel.com>
> ---
> Pekka, I guess now it's alright, please take a look. If it's good to go, you
> would mind to stamp a Reviewed-By for the other patches in this series as
> well?

Looks good, the only thing I'd done differently is make the function
take a typed parameter instead of a void*.

Oh, just realised...

> Thank you.
> 
> since v2:
> - adjusted to the correct limits based on stdint.h
> - instead drastically assert(0) on illegal user input, now it returns a
>   warning message and continue the execution of the program.
> 
>  shared/option-parser.c |   57 ++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 55 insertions(+), 2 deletions(-)
> 
> diff --git a/shared/option-parser.c b/shared/option-parser.c
> index 600f110..0e607d9 100644
> --- a/shared/option-parser.c
> +++ b/shared/option-parser.c
> @@ -25,18 +25,71 @@
>  #include <stdio.h>
>  #include <string.h>
>  #include <assert.h>
> +#include <limits.h>
> +#include <errno.h>
>  
>  #include "config-parser.h"
>  
> +static int
> +get_int(void *ret, const char *value)
> +{
> +	char *end;
> +	long int v;
> +
> +	errno = 0;
> +	v = strtol(value, &end, 0);
> +	if ((errno == ERANGE && (v == LONG_MAX || v == LONG_MIN)) ||
> +	    (errno != 0 && v == 0) ||
> +	    (end == value) ||
> +	    (*end != '\0')) {
> +		fprintf(stderr, "arg %s is not a valid number\n", value);
> +		return -1;
> +	}
> +
> +	if (v < INT32_MIN || v > INT32_MAX) {
> +		fprintf(stderr, "arg %s doesn't fit into integer\n", value);
> +		return -1;
> +	}
> +
> +	* (int32_t *) ret = v;
> +	return 1;
> +}
> +
> +static int
> +get_uint(void *ret, const char *value)
> +{
> +	char *end;
> +	long int v;
> +
> +	errno = 0;
> +	v = strtol(value, &end, 0);
> +	if ((errno == ERANGE && (v == ULONG_MAX)) ||
> +	    (errno != 0 && v == 0) ||
> +	    (end == value) ||
> +	    (*end != '\0')) {
> +		fprintf(stderr, "arg %s is not a valid number\n", value);
> +		return -1;
> +	}
> +
> +	if (v < 0 || v > UINT32_MAX) {
> +		fprintf(stderr, "arg %s doesn't fit into unsigned integer\n",
> +			value);
> +		return -1;
> +	}
> +
> +	* (uint32_t *) ret = v;
> +	return 1;
> +}
> +
>  static void
>  handle_option(const struct weston_option *option, char *value)
>  {
>  	switch (option->type) {
>  	case WESTON_OPTION_INTEGER:
> -		* (int32_t *) option->data = strtol(value, NULL, 0);
> +		get_int(option->data, value);
>  		return;
>  	case WESTON_OPTION_UNSIGNED_INTEGER:
> -		* (uint32_t *) option->data = strtoul(value, NULL, 0);
> +		get_uint(option->data, value);
>  		return;
>  	case WESTON_OPTION_STRING:
>  		* (char **) option->data = strdup(value);

...the error is not propagated forward here. But that could be another
patch.

this patch
Reviewed-by: Pekka Paalanen <ppaalanen at gmail.com>


Thanks,
pq