Re: [PATCH 2/4] Remove silent clamp of renameLimit

[Elijah Newren <newren@gmail.com>]

Thanks for taking a look!

On Fri, Nov 10, 2017 at 10:26 AM, Stefan Beller <sbeller@google.com> wrote:
<snip>
>> -       if (rename_limit <= 0 || rename_limit > 32767)
>> -               rename_limit = 32767;
>>         if ((num_create <= rename_limit || num_src <= rename_limit) &&
>> -           (num_create * num_src <= rename_limit * rename_limit))
>> +           ((double)num_create * (double)num_src
>> +            <= (double)rename_limit * (double)rename_limit))
>>                 return 0;
>
> From a technical perspective, I would think that if
> (num_create <= rename_limit || num_src <= rename_limit)
> holds true, that the double-cast condition would also be always true?
> Could we just remove that last check?

Not necessarily.  For example, if num_create = rename_limit-1 and
num_src = rename_limit+2, then the first condition will be satisfied
but the second won't.  If it was && rather than ||, then the second
condition would be superfluous.

> Or phrased differently, if we can cast to double and extend the check
> here, do we have to adapt code at other places as well?

Good point, and yes.  Perhaps I should have re-ordered my patch series
because I came back to it later and realized that the progress code
was broken due to overflow/wraparound, and a patch 3 fixed that.

Further, the later patch used uint64_t instead of double.  While
double works, perhaps I should change the double here to uint64_t for
consistency?