[PATCH 1/3] parser: be more pick for integer values

[Pekka Paalanen]

On Mon, 12 Mar 2012 19:06:38 -0300
Tiago Vignatti <tiago.vignatti at intel.com> wrote:

> It was accepting "-i=3", "-i=3/2", "--idle-time=*3" and similar unwanted type
> of arguments, giving wrong impression for the user. Now it explicitly ignores.
> 
> Signed-off-by: Tiago Vignatti <tiago.vignatti at intel.com>
> ---
> these three patches are here:
> http://cgit.freedesktop.org/~vignatti/weston/?h=options
> 
>  shared/option-parser.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 44 insertions(+), 0 deletions(-)
> 
> diff --git a/shared/option-parser.c b/shared/option-parser.c
> index 600f110..8b16b5d 100644
> --- a/shared/option-parser.c
> +++ b/shared/option-parser.c
> @@ -29,13 +29,57 @@
>  #include "config-parser.h"
>  
>  static void
> +check_int(const char *value)
> +{
> +	int count = 0, i = 1;
> +
> +	if (isdigit(value[0]) || value[0] == '-' || value[0] == '+')
> +		count++;
> +
> +	while (value[i]) {
> +		if (isdigit(value[i]))
> +			count++;
> +		i++;
> +	}
> +
> +	if (count != strlen(value)) {
> +		fprintf(stderr, "%s: invalid character in %s\n",
> +				__func__, value);
> +		assert(0);
> +	}
> +}
> +
> +static void
> +check_uint(const char *value)
> +{
> +	int count = 0, i = 1;
> +
> +	if (isdigit(value[0]) || value[0] == '+')
> +		count++;
> +
> +	while (value[i]) {
> +		if (isdigit(value[i]))
> +			count++;
> +		i++;
> +	}
> +
> +	if (count != strlen(value)) {
> +		fprintf(stderr, "%s: invalid character in %s\n",
> +				__func__, value);
> +		assert(0);
> +	}
> +}
> +
> +static void
>  handle_option(const struct weston_option *option, char *value)
>  {
>  	switch (option->type) {
>  	case WESTON_OPTION_INTEGER:
> +		check_int(value);
>  		* (int32_t *) option->data = strtol(value, NULL, 0);
>  		return;
>  	case WESTON_OPTION_UNSIGNED_INTEGER:
> +		check_uint(value);
>  		* (uint32_t *) option->data = strtoul(value, NULL, 0);
>  		return;
>  	case WESTON_OPTION_STRING:

Hi Tiago,

is there a reason you are not using strtol() built-in features for
checking?

If we want that the whole string 'value' must be a valid number in any
base, as an example:

char *end;

errno = 0;
long int v = strtol(value, &end, 0);

/* from the manual: */
if ((errno == ERANGE && (v == LONG_MAX || v == LONG_MIN)) ||
    (errno != 0 && v == 0) ||
    end == value ||
    *end != '\0') {
	printf("error: argument '%s' is not a valid number.\n", value);
	return;
}

if (v < INT_MIN || v > INT_MAX) {
	printf("error: argument '%s' does not fit into a 32-bit integer.\n", value);
	return;
}

*(int32_t *)option->data = v;

That way we accept all the usual number bases, accept leading white
space (trailing? dunno), and check that the whole conversion is
successful and not ignore trailing garbage. We also detect numbers that
cannot fit into the data types, both for conversion and assignment, and
complain about empty arguments.


Thanks,
pq