Re: [PATCH 3/5] pack-objects: clamp negative window size to 0

From: René Scharfe <l.s.r@web.de>
Date: May 4, 2021, 18:47 UTC
Message-ID: <02a66bfb-aac0-c05e-dab3-366bc312d900@web.de>
In-Reply-To: <YJAOzTIXkO2lhxSs@coredump.intra.peff.net>

Am 03.05.21 um 16:55 schrieb Jeff King:

Show 25 quoted lines

> On Mon, May 03, 2021 at 08:10:24AM -0400, Derrick Stolee wrote:
>
>> On 5/1/2021 10:03 AM, Jeff King wrote:
>>> A negative window size makes no sense, and the code in find_deltas() is
>>> not prepared to handle it. If you pass "-1", for example, we end up
>>> generate a 0-length array of "struct unpacked", but our loop assumes it
>>> has at least one entry in it (and we end up reading garbage memory).
>>>
>>> We could complain to the user about this, but it's more forgiving to
>>> just clamp it to 0, which means "do not find any deltas at all". The
>>> 0-case is already tested earlier in the script, so we'll make sure this
>>> does the same thing.
>>
>> This seems like a reasonable approach. It takes existing "undefined"
>> behavior and turns it into well-understood, "defined" behavior.
>
> I was on the fence on doing that, or just:
>
>   if (window < 0)
> 	die("sorry dude, negative windows are nonsense");
>
> So if anybody has a strong preference, I could be easily persuaded. Part
> of what led me to being forgiving was that we similarly clamp too-large
> depth values (with a warning; I didn't think it was really necessary
> here, though).

There's another option: Mapping -1 or all negative values to the maximum:

	if (window < 0)
		window = INT_MAX;
	if (depth < 0)
		depth = (1 << OE_DEPTH_BITS) - 1;

That's allows saying "gimme all you got" without knowing or being willing to type out the exact maximum value, which may change between versions. Not all that useful for --window, I guess. That convention has been used elsewhere I'm sure, but can't point out a concrete example. $arr[-1] get the last item of the array $arr in PowerShell, though, which is kind of similar.

Sure, you get the same effect in both cases by typing 2147483647, but -1 is more convenient.

Not a strong preference, but I thought it was at least worth mentioning that particular bike shed color. :)

René