Re: [PATCH] prio-queue: use size_t rather than int for size

From: Jeff King <peff@peff.net>
Date: Dec 27, 2024, 14:34 UTC
Message-ID: <20241227143440.GB88306@coredump.intra.peff.net>
In-Reply-To: <Z25-PaOQg7lAeyxx@pks.im>

On Fri, Dec 27, 2024 at 11:15:31AM +0100, Patrick Steinhardt wrote:

Show 9 quoted lines

> On Fri, Dec 20, 2024 at 03:49:49AM -0500, Jeff King wrote:
> [snip]
> >   4. The insertion_ctr variable is currently unsigned, but can likewise
> >      grow (it is actually worse, because adding and removing an element
> >      many times will keep increasing the counter, even though "nr" does
> >      not). I've bumped that to size_t here, as well.
> 
> I agree that bumping it to `size_t` would be sensible, but it doesn't
> look like you actually followed through with that change, as...

Doh, good catch. I had backed it out temporarily to confirm that it was a problem even before the patch, but accidentally ended up with the wrong state in the commit.

Show 5 quoted lines

> I was surprised by how few callers there are required any changes due to
> `.nr` changing. I scanned through the code and found one more instance
> where we have to adapt code, in commit-reach.c:queue_has_nonstale(). The
> compiler does not generate warnings for this case because the file isn't
> -Wsign-compare clean yet.

Makes sense.

> Anyway, I see that the patch has landed in "master" already. Let me send
> a follow-up patch series to fix both issues.

Thanks!

-Peff