Re: [PATCH v2 11/12] builtin/gc: fix signedness issues in ODB-related functionality

On Mon, Jul 13, 2026 at 09:28:02AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <[email protected]> writes:
> > diff --git a/builtin/gc.c b/builtin/gc.c
> > index 3207182488..8cf3781313 100644
> > --- a/builtin/gc.c
> > @@ -456,7 +458,7 @@ static struct packed_git *find_base_packs(struct odb_source_files *files,
> >  		if (e->pack->is_cruft)
> >  			continue;
> >  		if (limit) {
> > -			if (e->pack->pack_size >= limit)
> > +			if ((uintmax_t) e->pack->pack_size >= limit)
> 
> Here, just like in too_many_loose_objects(), 'limit' is of type
> 'unsigned long'.  While it makes sense to convert both sides of
> the comparison to an unsigned type, casting only the left side
> to a type that differs from the right side puzzles me.
> 
> Presumably, the other side is of type 'off_t', which is signed,
> explaining the desire to cast it to an unsigned type.  But I am
> not sure what happens if 'off_t' is wider than 'unsigned long'.

Yeah, `pack_size` is an `off_t`, which is signed. But we never populate
it with a negative value, so casting it to `uintmax_t` in unnecessary.
The right-hand side is already unsigned, so due to the usual arithmetic
conversion rules it would be automatically promoted to `uintmax_t`, as
well.

Patrick