Thread (10 messages) flat view 10 messages, 5 authors, 13d ago

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

From: Jeff King <hidden>
Date: 2026-07-26 07:41:02

On Fri, Jul 24, 2026 at 09:26:04AM -0700, Junio C Hamano wrote:
quoted
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.
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help