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

From: Elijah Newren <newren@gmail.com>
Date: Nov 11, 2017, 16:39 UTC
Message-ID: <CABPp-BHvveKW+UzTZqLq5h0bVUJLPqYpGu0CWQpYjHzzDbEWww@mail.gmail.com>
In-Reply-To: <20171110234210.lojdat5zofhrdlgi@genre.crustytoothpaste.net>

Hi,

On Fri, Nov 10, 2017 at 3:42 PM, brian m. carlson <sandals@crustytoothpaste.net> wrote:

> On Fri, Nov 10, 2017 at 10:36:17AM -0800, Elijah Newren wrote:

<snip>

Show 7 quoted lines

>> 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.

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.

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?