Re: [bugs] [PATCH] Fix integer parsing

On Tue, 28 Nov 2017 at 20:32:26, Lars Henriksen wrote:
> bug + bug = nobug?
> ---
> The 'is_all_digit(val + 1)' looked suspicious. If is_all_digit(NULL) returns
> true, "+" and "-" will be accepted as integers, if false, single-digit integers
> will be rejected. It does return true, but should return false.

I think you're mixing some things up. Firstly, I think this is about
calling is_all_digit() on an empty string, not on a NULL pointer.

The semantics should match the name of the function which roughly asks
"Is each of the characters in the string a digit?" or, equivalently,
"Does the input *not* contain any character which is *not* a digit?"

And, quite clearly, an empty string does not contain any character which
is not a digit, so the function should return true. I am also confused
as to why you think the previous implementation was faulty... More
below.

> 
>  src/config.c | 8 +++++---
>  src/utils.c  | 3 ++-
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/src/config.c b/src/config.c
> index c46d718..f8eee98 100644
> --- a/src/config.c
> +++ b/src/config.c
> @@ -151,12 +151,14 @@ static int config_parse_unsigned(unsigned *dest, const char *val)
>  
>  static int config_parse_int(int *dest, const char *val)
>  {
> -       if ((*val == '+' || *val == '-' || isdigit(*val))
> -           && is_all_digit(val + 1))

If the value is a single digit, isdigit(*val) is true and is_all_digit()
is invoked with an empty string (which should also return true). So
everything is as it should be...?

Regards,
Lukas

Links