Re: [PATCH v3 5/8] commit-reach: introduce struct paint_state with per-side counters

From: René Scharfe <l.s.r@web.de>
Date: Jun 26, 2026, 21:13 UTC
Message-ID: <bd37b80d-9eff-496d-8f1f-436594968678@web.de>
In-Reply-To: <e82e0c72b6fc72b214f40efa9586c77790881f93.1782479286.git.gitgitgadget@gmail.com>

On 6/26/26 3:08 PM, Kristofer Karlsson via GitGitGadget wrote:

Show 20 quoted lines

> 
> diff --git a/commit-reach.c b/commit-reach.c
> index f6a438550b..0f29b143bd 100644
> --- a/commit-reach.c
> +++ b/commit-reach.c
> @@ -97,6 +97,75 @@ static struct commit *nonstale_queue_get_dedup(struct nonstale_queue *queue)
>  	return commit;
>  }
>  
> +/*
> + * Priority queue with per-side commit counters for paint_down_to_common().
> + * Each non-stale queued commit occupies exactly one bucket: PARENT1-only,
> + * PARENT2-only, or both (a pending merge-base candidate).
> + */
> +struct paint_state {
> +	struct prio_queue queue;
> +	int p1_count;
> +	int p2_count;
> +	int pending_merge_bases;
> +};

Can they become negative? Wouldn't size_t be a more natural fit, matching nr from struct prio_queue?

And some bikeshedding:

Why abbreviate? parent1_count and parent2_count would be slightly easier to read and associate with PARENT1 and PARENT2.

And pending_merge_bases is a counter as well. Why not call it like that, pending_merge_base_count? Well, that's pretty long. both_count? That's quite generic and nondescript. Call the other counters parents1 and parents2? Nah. Or parent1s and parent2s? Not sure why this inconsistency bothers me to begin with.

René