Re: [PATCH v3 2/4] pack-bitmap: fix bug with exact ref match in "pack.preferBitmapTips"

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 6, 2026, 17:18 UTC
Message-ID: <xmqq343dhjt3.fsf@gitster.g>
In-Reply-To: <20260206-b4-pks-fix-for-each-ref-in-misuse-v3-2-1e050c3d6a50@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

Show 19 quoted lines

> @@ -3328,15 +3328,26 @@ void for_each_preferred_bitmap_tip(struct repository *repo,
>  {
>  	struct string_list_item *item;
>  	const struct string_list *preferred_tips;
> +	struct strbuf buf = STRBUF_INIT;
>  
>  	preferred_tips = bitmap_preferred_tips(repo);
>  	if (!preferred_tips)
>  		return;
>  
>  	for_each_string_list_item(item, preferred_tips) {
> +		const char *pattern = item->string;
> +
> +		if (!ends_with(pattern, "/")) {
> +			strbuf_reset(&buf);
> +			strbuf_addf(&buf, "%s/", pattern);
> +			pattern = buf.buf;
> +		}
> +

I briefly wondered if a possible alternative solution is to update bitmap_preferred_tips() that reads the configuration so that it gives the callers a string-list with "corrected" strings. If it can have many other callers, such an approach would force everybody to adopt the same worldview, i.e., the configuration is meant to specify hierarchy prefixes and never individual refs.

But when I noticed that nobody bothers to release resources held by preferred_Tips, I realized that the above implementation is far simpler and much better. This iterator is the only caller of the file-scope static bitmap_preferred_tips() helper, and what the helper returns is a borrowed piece of string_list that is owned by the config subsystem, so such a change will force us to make a copy and then release the copy in the caller.

So, I think the above change is much better than such an alternative.

Thanks.