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

From: Patrick Steinhardt <ps@pks.im>
Date: Dec 27, 2024, 10:15 UTC
Message-ID: <Z25-PaOQg7lAeyxx@pks.im>
In-Reply-To: <20241220084949.GA132704@coredump.intra.peff.net>

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

Show 7 quoted lines

> diff --git a/prio-queue.h b/prio-queue.h
> index 4f9a37e6be..36f370625f 100644
> --- a/prio-queue.h
> +++ b/prio-queue.h
> @@ -30,7 +30,7 @@ struct prio_queue {
>  	prio_queue_compare_fn compare;
>  	unsigned insertion_ctr;

... it still says `unsigned` here.

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.

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

Patrick