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

[Junio C Hamano <gitster@pobox.com>]

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.

Sorry about the thinko, and thanks for a new and interesting
tangent.