Re: [PATCH 2/4] Remove silent clamp of renameLimit
- From
brian m. carlson <sandals@crustytoothpaste.net>
- Date
- Nov 10, 2017, 23:42 UTC
- Message-ID
- <20171110234210.lojdat5zofhrdlgi@genre.crustytoothpaste.net>
- In-Reply-To
- <CABPp-BGwoduH_T7U4uN5A-h43wREfXdQgV8YCVWf8w5N09cYSw@mail.gmail.com>
On Fri, Nov 10, 2017 at 10:36:17AM -0800, Elijah Newren wrote:
Show 23 quoted lines
> Thanks for taking a look! > > On Fri, Nov 10, 2017 at 10:26 AM, Stefan Beller <sbeller@google.com> wrote: > > 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?
I'm wondering if maybe you want to use size_t. If you end up using an unsigned type, you might be able to leverage unsigned_mult_overflows to avoid having to write this by hand.
-- brian m. carlson / brian with sandals: Houston, Texas, US https://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: https://keybase.io/bk2204