From: Jeff King <peff@peff.net>
Date: Mon, 23 Feb 2026 13:49:35 GMT
Subject: Re: [PATCH v2 11/18] git-compat-util.h: introduce `u32_add()`
Message-ID: <20260223134935.GA271392@coredump.intra.peff.net>
In-Reply-To: <aXFni2tE7vn1dKFp@nand.local>

On Wed, Jan 21, 2026 at 06:55:55PM -0500, Taylor Blau wrote:

> I had considered this approach when writing, but ultimately decided
> against it, since it felt a little clunky to have to pass a pointer in
> to do a simple arithmetic operation. But I think your point about
> ensuring that we actually do:
> 
>     if (unsigned_mult_overflows(a, b))
>       die(...);
>     result = a * b;
> 
> and not "result = a * c" or some other expression which is not "a * b"
> is a good one.

It is clunky, but it's how the compiler intrinsics work (if we ever
chose to use them).

> I dunno. The spots in this patch are the only uses of u64_mult() and
> u64_add(), so I'm hesitant to keep a helper function around just for
> that sole use-case. I wonder if we should do what you suggest here for
> the much more frequently used st_add() / st_mult() / st_sub() functions?

I am to blame for the crappy interface of the st_add() etc functions. It
did make conversion _much_ easier, because you can do stuff like:

  -foo = malloc(nr * size);
  +foo = malloc(st_mult(nr, size));

as opposed to:

  size_t total;
  ...
  st_mult(&total, nr, size));
  foo = malloc(total);

My rationale was that size_t computations like this are OK to die() with
very little useful error reporting up the chain because:

  1. The result is generally just passed along to malloc() anyway, where
     we likewise find it OK to die() without much info. So you can
     imagine a world where we just do 128-bit size computations and then
     let malloc() fail, and it would look the same. ;)

  2. They don't happen in practice unless there is a bug or a malicious
     input. Which is mostly true for 64-bit systems. Maybe less so for
     32-bit ones, where you might conceivably wish to have 4 billion of
     something.

I don't think any of that holds true for u32 values like counts of
objects. It's conceivable that you might want to try to write a midx for
two packs with 2.1 billion objects each (though from my experience, such
a repo would be unusable).

Anyway. My point is mostly that I think we can design u32_add() to be
what we want and not worry too much about going back to fix st_add(),
etc.

-Peff