Re: [PATCH 3/3] git_parse_signed(): avoid integer overflow

> René Scharfe <l.s.r@web.de> writes:
>
>>>> +		if (max < 0)
>>>> +			BUG("max must be a positive integer");
>>>
>>> In parse_signed(), would we expect to accept end-user input that is
>>> a negative integer?  We must.  Otherwise we would not be calling a
>>> "signed" parser.  Now, are there cases where the valid value range
>>> is bounded by a negative integer at the top?  No current callers may
>>> pass such a value, but is it reasonable to add such a new constraints
>>> to an existing API function?
>>
>> Hmm, if minimum and maximum are not symmetric, then we need to supply
>> both, don't we?
>
> Ah, thanks for injecting doze of sanity---I totally missed that the
> bound was about the absolute value, so we can say "this is signed,
> and the allowed values are (-3, -2, -1, 0, 1, 2, 3).  If so, then the
> "reject negative max" in the posted patch is not a problem as I said
> above.  I somehow thought that giving -1 as "max" would allow callers
> to say "non-negative numbers are not allowed".  But that is not what
> is going on.

> Allowing callers to specify both lower and uppoer bounds so that
> they can say "the allowed values are (-1, 0, 1, 2, 3)", while it
> might make it more useful, is a separate new feature development and
> outside the scope of "let's tighten the parsing of end user input"
> Phillip has here.