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

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

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