Re: [PATCH 2/4] Remove silent clamp of renameLimit
- Date: Fri, 10 Nov 2017 10:36:17 -0800
- From: Elijah Newren <newren@xxxxxxxxx>
- Subject: Re: [PATCH 2/4] Remove silent clamp of renameLimit
Thanks for taking a look!
On Fri, Nov 10, 2017 at 10:26 AM, Stefan Beller <sbeller@xxxxxxxxxx> wrote:
>> - 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