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

From: Jeff King <peff@peff.net>
Date: Jul 26, 2026, 07:41 UTC
Message-ID: <20260726074100.GA2366012@coredump.intra.peff.net>
In-Reply-To: <xmqqbjbw8icj.fsf@gitster.g>

On Fri, Jul 24, 2026 at 09:26:04AM -0700, Junio C Hamano wrote:

Show 8 quoted lines

> > 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.
> 
> Having said that, another fix might be to standardize the way we
> count the number of things in an array and update 'ref-filter.h' to
> use size_t in 'struct ref_array' as well.

Yes, I had the same thought.

I am generally in favor of using size_t for anything that counts allocations. I'd also be fine with (and maybe even prefer) a type that is a signed integer of the same magnitude as size_t, because loops, etc, are often easier to reason about when "0 - 1" is actually less than 0, and doesn't wrap. But we would need to define our own custom type for that, since ssize_t isn't portable enough.

Show 6 quoted lines

> It is not as though 2 billion refs are too few to satisfy our
> needs, and in general, the platform-natural int should be used to
> count things unless there is a compelling reason to deviate from
> that norm.  However, "somehow we ended up counting many things in
> size_t, so it is better to count everything using the same type"
> could serve as "the compelling reason" to make such a change.

Yeah, I think that consistency is nice.

My personal reason (and this is mostly re-hashing previous discussions) is avoiding integer overflow attacks by making it impractical to allocate sufficient memory.

If you had a repository with 3 billion refs, then I think right now "git for-each-ref" would wrap and start using negative values. I _suspect_ it would be caught when ALLOC_GROW() converts that negative into to a size_t (yielding an impractical allocation), but I don't think it's practical to try. I started feeding 2^31 refs into "update-ref --stdin" and it was around 64GB of heap after only 160 million or so.

But in general, if the counters are all size_t or similar magnitude, then any geometric growth pattern is going to require allocating some significant portion of the whole address space before we hit the integer overflow condition (and presumably such an allocation would fail).

-Peff