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

From: brian m. carlson <sandals@crustytoothpaste.net>
Date: Nov 11, 2017, 17:32 UTC
Message-ID: <20171111173253.idmyypjevwqblica@genre.crustytoothpaste.net>
In-Reply-To: <CABPp-BHvveKW+UzTZqLq5h0bVUJLPqYpGu0CWQpYjHzzDbEWww@mail.gmail.com>

On Sat, Nov 11, 2017 at 08:39:11AM -0800, Elijah Newren wrote:

Show 6 quoted lines

> Thanks for pointing out unsigned_mult_overflows; I was unaware of it.
> I think I'd prefer to not use it though; the fact that I had a case
> that genuinely needed a value greater than 2^31 (at least before my
> performance patches) meant that a slightly bigger repo is going to
> eventually need a value over 2^32, so I think we should just cast to a
> type that can hold it.

That's fine. I had considered this in the context of 64-bit values, but I suppose that the likelihood of us hitting 2**64 iterations (and performing reasonably as well) is unlikely.

> I'm curious why you suggest size_t, though.  I have always associated
> that with an amount of memory that will be used, and there's no
> allocation based on this result.  Was I wrong to make that
> association, or is there another good reason here?

I usually like size_t for values that are unsigned and don't need to be fixed size because it's usually the largest efficient unsigned type. However, if you want something to handle items larger than 2**32, then I agree that it's maybe not a great idea if we want it to work on 32-bit systems.

-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204