Hi Bernd,

> -static long
> +static int
>  d_compact_number (struct d_info *di)
>  {
> -  long num;
> +  int num;
>    if (d_peek_char (di) == '_')
>      num = 0;
>    else if (d_peek_char (di) == 'n')
> @@ -2957,7 +2957,7 @@ d_compact_number (struct d_info *di)
>    else
>      num = d_number (di) + 1;
> 
> -  if (! d_check_char (di, '_'))
> +  if (num < 0 || ! d_check_char (di, '_'))
>      return -1;
>    return num;
>  }

> Shouldn't we check for overflows before performing the +1 addition (i.e. 0 <= 
> num < INT_MAX)? Ideally we'd also have a way to signal from d_number if we 
> had an overflow while parsing that number.
Without an overflow signal, d_number will already be prone to return a negative 
number for supposedly non-negative numbers (those not preceded with ’n’). In 
that case an overflow check would be unnecessary in d_compact_number which is 
supposed to always return a positive number or a negative one (-1). If you 
decide in favour of an overflow signal, it must be handled by the call-sites. 
Not sure what the “default behaviour” should be then. Otherwise, we can simply 
assume that the call sites for d_number can handle negative numbers.

Kindly advice.

> 
> There's also this, in d_expression_1:
> 
>         index = d_compact_number (di) + 1;
>         if (index == 0)
>           return NULL;
> 
> which probably ought to have the same kind of check (I'll note that at this 
> point we've accumulated two "+1"s, I'll assume that's what we want).
Yes. There should be an overflow check here. 

Thanks!
- Marcel

Reply via email to