From: Taylor Blau <me@ttaylorr.com>
Date: Tue, 09 Dec 2025 02:13:54 GMT
Subject: Re: [PATCH 10/17] git-compat-util.h: introduce `u32_add()`
Message-ID: <aTeF4rn6h1gc6Kgl@nand.local>
In-Reply-To: <aTcYdebOuUfrAAJJ@pks.im>

On Mon, Dec 08, 2025 at 07:27:01PM +0100, Patrick Steinhardt wrote:
> On Sat, Dec 06, 2025 at 03:31:28PM -0500, Taylor Blau wrote:
> > diff --git a/git-compat-util.h b/git-compat-util.h
> > index 398e0fac4fa..a7aa5f05fc9 100644
> > --- a/git-compat-util.h
> > +++ b/git-compat-util.h
> > @@ -670,6 +670,14 @@ static inline int cast_size_t_to_int(size_t a)
> >  	return (int)a;
> >  }
> >
> > +static inline uint32_t u32_add(uint32_t a, uint32_t b)
> > +{
> > +	if (unsigned_add_overflows(a, b))
> > +		die("uint32_t overflow: %"PRIuMAX" + %"PRIuMAX,
> > +		    (uintmax_t)a, (uintmax_t)b);
> > +	return a + b;
> > +}
>
> We already use PRIu32 in our codebase, so why is the cast necessary?

I don't think it is; we could easily write this as:

    die("uint32_t overflow: %"PRIu32" + %"PRIu32, a, b);

instead, but this matches the convention of other similar functions in
the compat-util header.

(It's possible that there is some reasoning here that using PRIuMAX
really *is* necessary, but it isn't clear to me that's the case.)

Thanks,
Taylor