git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] branch: avoid slow strvec Coccinelle matching

From
Junio C Hamano <gitster@pobox.com>
Date
Jul 24, 2026, 15:58 UTC
Message-ID
<xmqqpl0c8jml.fsf@gitster.g>
In-Reply-To
<20260724114948.GA825505@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 30 quoted lines
> The static-analysis CI job uses the ubuntu-22.04 image, for no reason
> that I can really discern. It looks like coccinelle 1.3.0 is in ubuntu
> 25.10, according to:
>
>   https://packages.ubuntu.com/km/questing/coccinelle
>
> Why don't we just use the more recent version instead of trying to work
> around it? That would fix this problem and prevent future ones. Looking
> at the code in question:
>
>> diff --git a/builtin/branch.c b/builtin/branch.c
>> index 42f2221547..2415a275ea 100644
>> --- a/builtin/branch.c
>> +++ b/builtin/branch.c
>> @@ -797,10 +797,9 @@ static int delete_merged_branches(const struct strvec *upstreams,
>>  	struct strbuf key = STRBUF_INIT;
>>  	struct hashmap_iter iter;
>>  	struct strmap_entry *entry;
>> -	size_t i;
>>  	int ret = 0;
>>  
>> -	for (i = 0; i < upstreams->nr; i++)
>> +	for (size_t i = 0; i < upstreams->nr; i++)
>>  		if (ref_filter_forked_add(&filter, upstreams->v[i]) < 0)
>>  			die(_("'%s' is not a valid branch or pattern"),
>>  			    upstreams->v[i]);
>
> ...there is nothing suspicious or wrong about it. It seems likely that
> somebody else may end up writing something similar and triggering the
> same problem.
Exactly.
> That said, moving the iterator into the loop declaration is perhaps
> nicer anyway, because it avoids two unrelated uses of the same variable.
Exactly again.
Show 20 quoted lines
> Notably:
>
>> @@ -809,7 +808,7 @@ static int delete_merged_branches(const struct strvec *upstreams,
>>  	filter.name_patterns = argv;
>>  	filter_refs(&candidates, &filter, filter.kind);
>>  
>> -	for (i = 0; i < (size_t)candidates.nr; i++) {
>> +	for (size_t i = 0; i < (size_t)candidates.nr; i++) {
>>  		const char *branch_refname = candidates.items[i]->refname;
>>  		const char *branch_name;
>>  		struct branch *branch;
>
> This hunk is not using a strvec at all. Because it uses the same
> variable, if we did not change this loop, then we'd still have to
> declare "i" at the top of the function and the other loop would
> introduce a shadowed variable. That's not wrong, but it is confusing.
>
> However, if we are going to have our own variable here, perhaps it
> should use the correct type? candidate.nr is an int, so probably this
> should also be an int, and then the gross cast can go away.

Ah, very good eyes. It is a disease to try appeasing -Wsign-compare without thinking, instead of questioning the value of the warning first, and in this case there is no reason to try forcing the use of size_t, even with the unnecessary casting.

Previous: Harald NordgrenNext: Junio C Hamano
Message 4 of 11 in “branch: avoid slow strvec Coccinelle matching”
  1. branch: avoid slow strvec Coccinelle matchingtnyman@openai.com, Jul 24, 2026
  2. Jeff KingJul 24, 2026
  3. Harald NordgrenJul 24, 2026
  4. Junio C HamanoJul 24, 2026
  5. Junio C HamanoJul 24, 2026
  6. Junio C HamanoJul 24, 2026
  7. Jeff KingJul 26, 2026
  8. Emmanuel UgwuSep 4, 2026
  9. Jeff KingJul 26, 2026
  10. Junio C HamanoJul 24, 2026
  11. Taylor BlauJul 24, 2026

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.