On Fri, Nov 10, 2017 at 10:36:17AM -0800, Elijah Newren wrote: > Thanks for taking a look! > > On Fri, Nov 10, 2017 at 10:26 AM, Stefan Beller <sbeller@xxxxxxxxxx> 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
Attachment:
signature.asc
Description: PGP signature
