From: Taylor Blau Date: Thu, 15 Jan 2026 00:11:11 GMT Subject: Re: [PATCH v2 11/18] git-compat-util.h: introduce `u32_add()` Message-ID: In-Reply-To: On Wed, Jan 14, 2026 at 05:03:56PM -0500, Taylor Blau wrote: > As for removing u64_add(), that should be straightforward as well since > there is also a single caller. Let me know if you think that makes sense > to take up as part of this series, or if you would prefer it done > separately. I tend to prefer the latter, since the state after applying > the above is that we avoid adding any new callers. This appears to be easy enough. The following applies on top of 'master' if you want to pick it up separately: --- 8< --- Subject: [PATCH] git-compat-util.h: drop u64_add(), u64_mult() helpers The u64_add() and u64_mult() helper functions were introduced in b103881d4f4 (midx repack: avoid integer overflow on 32 bit systems, 2025-05-22) to implement overflow checks during a fixed-point calculation when estimating pack sizes in the MIDX writing code. However, those functions call die() when either the addition or multiplication of their operands (depending on which function is being called) would cause an overflow. This does not allow the caller to provide a more detailed message, presenting the user with an opaque message like: fatal: uint64_t overflow: M * N Let's discourage these opaque error messages by dropping these functions entirely and instead having the caller use unsigned_mult_overflows() or unsigned_add_overflows() themselves, providing the caller the opportunity to come up with their own die() message. Suggested-by: Junio C Hamano Signed-off-by: Taylor Blau --- git-compat-util.h | 16 ---------------- midx-write.c | 15 +++++++++++++-- 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/git-compat-util.h b/git-compat-util.h index b0673d1a450..24edd68c671 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -641,22 +641,6 @@ static inline int cast_size_t_to_int(size_t a) return (int)a; } -static inline uint64_t u64_mult(uint64_t a, uint64_t b) -{ - if (unsigned_mult_overflows(a, b)) - die("uint64_t overflow: %"PRIuMAX" * %"PRIuMAX, - (uintmax_t)a, (uintmax_t)b); - return a * b; -} - -static inline uint64_t u64_add(uint64_t a, uint64_t b) -{ - if (unsigned_add_overflows(a, b)) - die("uint64_t overflow: %"PRIuMAX" + %"PRIuMAX, - (uintmax_t)a, (uintmax_t)b); - return a + b; -} - /* * Limit size of IO chunks, because huge chunks only cause pain. OS X * 64-bit is buggy, returning EINVAL if len >= INT_MAX; and even in diff --git a/midx-write.c b/midx-write.c index 87b97c70872..6006b6569c8 100644 --- a/midx-write.c +++ b/midx-write.c @@ -1738,8 +1738,19 @@ static void fill_included_packs_batch(struct repository *r, */ expected_size = (uint64_t)pack_info[i].referenced_objects << 14; expected_size /= p->num_objects; - expected_size = u64_mult(expected_size, p->pack_size); - expected_size = u64_add(expected_size, 1u << 13) >> 14; + + if (unsigned_mult_overflows(expected_size, + (uint64_t)p->pack_size)) + die(_("overflow during fixed-point multiply (%"PRIu64" " + "* %"PRIu64")"), + expected_size, (uint64_t)p->pack_size); + expected_size = expected_size * p->pack_size; + + if (unsigned_add_overflows(expected_size, 1u << 13)) + die(_("overflow during fixed-point rounding (%"PRIu64" " + " + %"PRIu64")"), + expected_size, (uint64_t)(1ul << 13)); + expected_size = (expected_size + (1u << 13)) >> 14; if (expected_size >= batch_size) continue; base-commit: 8745eae506f700657882b9e32b2aa00f234a6fb6 -- 2.52.0.436.g7dc2c5478ff