Re: [PATCH v2 11/18] git-compat-util.h: introduce `u32_add()`

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 14, 2026, 21:49 UTC
Message-ID: <xmqqpl7beugj.fsf@gitster.g>
In-Reply-To: <c0c1769464b1c8065c2cea59dfd85a1d37de9dd1.1768420450.git.me@ttaylorr.com>

Taylor Blau <me@ttaylorr.com> writes:

Show 24 quoted lines

> A future commit will want to add two 32-bit unsigned values together
> while checking for overflow. Introduce a variant of the u64_add()
> function for operating on 32-bit inputs.
>
> Signed-off-by: Taylor Blau <me@ttaylorr.com>
> ---
>  git-compat-util.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/git-compat-util.h b/git-compat-util.h
> index b0673d1a450..db62a6f25c5 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -641,6 +641,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;
> +}

Neither this one, nor the original u64_add(), seem to me a particularly good API.

When things might overflow, it is a given that we should give a controlled death rather than nonsense behaviour and/or corrupt output, but shouldn't the diagnosis message given to the end-user when we find an overflow be given at a bit higher layer? At this level, you do not even know what quantities you are adding together *means*. Even though your caller might be able to give a more intelligible message like "the number of packs to combine exceeds 2^32 that is too many". But dying in these functions means the callers have no chance to do so. The only thing the user sees is some code tried to add two u32 and overflowed---without any hint what these quantities were or what the addition was trying to compute.