Am 03.05.21 um 16:55 schrieb Jeff King:
> 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é
