public inbox for git@vger.kernel.org
 help / color / mirror / Atom feed
From: Taylor Blau <me@ttaylorr.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Jeff King <peff@peff.net>,
	Elijah Newren <newren@gmail.com>, Patrick Steinhardt <ps@pks.im>
Subject: Re: [PATCH v2 11/18] git-compat-util.h: introduce `u32_add()`
Date: Wed, 14 Jan 2026 19:11:11 -0500	[thread overview]
Message-ID: <aWgwn2rk/qw+fRoA@nand.local> (raw)
In-Reply-To: <aWgSzI30k0BZfZ4Q@nand.local>

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 <gitster@pobox.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 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

  reply	other threads:[~2026-01-15  0:11 UTC|newest]

Thread overview: 99+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-06 20:30 [PATCH 00/17] midx: incremental MIDX/bitmap layer compaction Taylor Blau
2025-12-06 20:31 ` [PATCH 01/17] midx: mark `get_midx_checksum()` arguments as const Taylor Blau
2025-12-08 18:26   ` Patrick Steinhardt
2025-12-09  1:41     ` Taylor Blau
2025-12-06 20:31 ` [PATCH 02/17] midx: split `get_midx_checksum()` by adding `get_midx_hash()` Taylor Blau
2025-12-08 18:25   ` Patrick Steinhardt
2025-12-09  1:42     ` Taylor Blau
2025-12-09  1:50       ` Taylor Blau
2025-12-09  6:27         ` Patrick Steinhardt
2026-01-13 22:46           ` Taylor Blau
2025-12-06 20:31 ` [PATCH 03/17] builtin/multi-pack-index.c: make '--progress' a common option Taylor Blau
2025-12-06 20:31 ` [PATCH 04/17] git-multi-pack-index(1): remove non-existent incompatibility Taylor Blau
2025-12-06 20:31 ` [PATCH 05/17] git-multi-pack-index(1): align SYNOPSIS with 'git multi-pack-index -h' Taylor Blau
2025-12-06 20:31 ` [PATCH 06/17] t/t5319-multi-pack-index.sh: fix copy-and-paste error in t5319.39 Taylor Blau
2025-12-06 20:31 ` [PATCH 07/17] midx-write.c: don't use `pack_perm` when assigning `bitmap_pos` Taylor Blau
2025-12-08 18:26   ` Patrick Steinhardt
2025-12-09  1:59     ` Taylor Blau
2025-12-06 20:31 ` [PATCH 08/17] midx-write.c: introduce `struct write_midx_opts` Taylor Blau
2025-12-08 18:26   ` Patrick Steinhardt
2025-12-09  2:04     ` Taylor Blau
2025-12-06 20:31 ` [PATCH 09/17] midx: do not require packs to be sorted in lexicographic order Taylor Blau
2025-12-08 18:26   ` Patrick Steinhardt
2025-12-09  2:07     ` Taylor Blau
2025-12-09  2:11       ` Taylor Blau
2025-12-06 20:31 ` [PATCH 10/17] git-compat-util.h: introduce `u32_add()` Taylor Blau
2025-12-08 18:27   ` Patrick Steinhardt
2025-12-09  2:13     ` Taylor Blau
2025-12-06 20:31 ` [PATCH 11/17] midx-write.c: introduce `midx_pack_perm()` helper Taylor Blau
2025-12-06 20:31 ` [PATCH 12/17] midx-write.c: extract `fill_pack_from_midx()` Taylor Blau
2025-12-06 20:31 ` [PATCH 13/17] midx-write.c: enumerate `pack_int_id` values directly Taylor Blau
2025-12-08 18:27   ` Patrick Steinhardt
2025-12-09  2:14     ` Taylor Blau
2025-12-06 20:31 ` [PATCH 14/17] midx-write.c: factor fanout layering from `compute_sorted_entries()` Taylor Blau
2025-12-06 20:31 ` [PATCH 15/17] t/helper/test-read-midx.c: plug memory leak when selecting layer Taylor Blau
2025-12-08 18:27   ` Patrick Steinhardt
2025-12-09  2:16     ` Taylor Blau
2025-12-06 20:31 ` [PATCH 16/17] midx: implement MIDX compaction Taylor Blau
2025-12-09  7:21   ` Patrick Steinhardt
2026-01-13 23:32     ` Taylor Blau
2025-12-06 20:31 ` [PATCH 17/17] midx: enable reachability bitmaps during " Taylor Blau
2025-12-09  7:21   ` Patrick Steinhardt
2026-01-13 23:47     ` Taylor Blau
2026-01-14 19:54 ` [PATCH v2 00/18] midx: incremental MIDX/bitmap layer compaction Taylor Blau
2026-01-14 19:54   ` [PATCH v2 01/18] midx: mark `get_midx_checksum()` arguments as const Taylor Blau
2026-01-14 19:54   ` [PATCH v2 02/18] midx: rename `get_midx_checksum()` to `midx_get_checksum_hash()` Taylor Blau
2026-01-14 19:54   ` [PATCH v2 03/18] midx: introduce `midx_get_checksum_hex()` Taylor Blau
2026-01-14 19:54   ` [PATCH v2 04/18] builtin/multi-pack-index.c: make '--progress' a common option Taylor Blau
2026-01-14 19:54   ` [PATCH v2 05/18] git-multi-pack-index(1): remove non-existent incompatibility Taylor Blau
2026-01-14 19:54   ` [PATCH v2 06/18] git-multi-pack-index(1): align SYNOPSIS with 'git multi-pack-index -h' Taylor Blau
2026-01-14 19:54   ` [PATCH v2 07/18] t/t5319-multi-pack-index.sh: fix copy-and-paste error in t5319.39 Taylor Blau
2026-01-14 19:54   ` [PATCH v2 08/18] midx-write.c: don't use `pack_perm` when assigning `bitmap_pos` Taylor Blau
2026-01-14 21:13     ` Junio C Hamano
2026-01-14 21:40       ` Taylor Blau
2026-01-14 19:54   ` [PATCH v2 09/18] midx-write.c: introduce `struct write_midx_opts` Taylor Blau
2026-01-14 19:54   ` [PATCH v2 10/18] midx: do not require packs to be sorted in lexicographic order Taylor Blau
2026-01-14 21:28     ` Junio C Hamano
2026-01-14 21:44       ` Taylor Blau
2026-01-27  7:34     ` Patrick Steinhardt
2026-02-24 18:47       ` Taylor Blau
2026-01-14 19:54   ` [PATCH v2 11/18] git-compat-util.h: introduce `u32_add()` Taylor Blau
2026-01-14 21:49     ` Junio C Hamano
2026-01-14 22:03       ` Taylor Blau
2026-01-15  0:11         ` Taylor Blau [this message]
2026-01-21  8:51           ` Patrick Steinhardt
2026-01-21 23:55             ` Taylor Blau
2026-01-22  2:26               ` rsbecker
2026-01-22 17:07                 ` Junio C Hamano
2026-02-23 13:49               ` Jeff King
2026-02-24 18:53                 ` Taylor Blau
2026-01-14 19:54   ` [PATCH v2 12/18] midx-write.c: introduce `midx_pack_perm()` helper Taylor Blau
2026-01-14 19:54   ` [PATCH v2 13/18] midx-write.c: extract `fill_pack_from_midx()` Taylor Blau
2026-01-14 19:54   ` [PATCH v2 14/18] midx-write.c: enumerate `pack_int_id` values directly Taylor Blau
2026-01-14 19:55   ` [PATCH v2 15/18] midx-write.c: factor fanout layering from `compute_sorted_entries()` Taylor Blau
2026-01-14 19:55   ` [PATCH v2 16/18] t/helper/test-read-midx.c: plug memory leak when selecting layer Taylor Blau
2026-01-14 19:55   ` [PATCH v2 17/18] midx: implement MIDX compaction Taylor Blau
2026-01-27  7:35     ` Patrick Steinhardt
2026-01-27 22:13       ` Taylor Blau
2026-01-14 19:55   ` [PATCH v2 18/18] midx: enable reachability bitmaps during " Taylor Blau
2026-02-20 22:24   ` [PATCH v2 00/18] midx: incremental MIDX/bitmap layer compaction Junio C Hamano
2026-02-23 14:08     ` Jeff King
2026-02-24  5:25       ` Taylor Blau
2026-02-24 18:59 ` [PATCH v3 00/17] " Taylor Blau
2026-02-24 18:59   ` [PATCH v3 01/17] midx: mark `get_midx_checksum()` arguments as const Taylor Blau
2026-02-24 18:59   ` [PATCH v3 02/17] midx: rename `get_midx_checksum()` to `midx_get_checksum_hash()` Taylor Blau
2026-02-24 18:59   ` [PATCH v3 03/17] midx: introduce `midx_get_checksum_hex()` Taylor Blau
2026-02-24 18:59   ` [PATCH v3 04/17] builtin/multi-pack-index.c: make '--progress' a common option Taylor Blau
2026-02-24 18:59   ` [PATCH v3 05/17] git-multi-pack-index(1): remove non-existent incompatibility Taylor Blau
2026-02-24 18:59   ` [PATCH v3 06/17] git-multi-pack-index(1): align SYNOPSIS with 'git multi-pack-index -h' Taylor Blau
2026-02-24 19:00   ` [PATCH v3 07/17] t/t5319-multi-pack-index.sh: fix copy-and-paste error in t5319.39 Taylor Blau
2026-02-24 19:00   ` [PATCH v3 08/17] midx-write.c: don't use `pack_perm` when assigning `bitmap_pos` Taylor Blau
2026-02-24 19:00   ` [PATCH v3 09/17] midx-write.c: introduce `struct write_midx_opts` Taylor Blau
2026-02-24 19:00   ` [PATCH v3 10/17] midx: do not require packs to be sorted in lexicographic order Taylor Blau
2026-02-24 19:00   ` [PATCH v3 11/17] midx-write.c: introduce `midx_pack_perm()` helper Taylor Blau
2026-02-24 19:00   ` [PATCH v3 12/17] midx-write.c: extract `fill_pack_from_midx()` Taylor Blau
2026-02-24 19:00   ` [PATCH v3 13/17] midx-write.c: enumerate `pack_int_id` values directly Taylor Blau
2026-02-24 19:00   ` [PATCH v3 14/17] midx-write.c: factor fanout layering from `compute_sorted_entries()` Taylor Blau
2026-02-24 19:00   ` [PATCH v3 15/17] t/helper/test-read-midx.c: plug memory leak when selecting layer Taylor Blau
2026-02-24 19:00   ` [PATCH v3 16/17] midx: implement MIDX compaction Taylor Blau
2026-02-24 19:00   ` [PATCH v3 17/17] midx: enable reachability bitmaps during " Taylor Blau

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aWgwn2rk/qw+fRoA@nand.local \
    --to=me@ttaylorr.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=newren@gmail.com \
    --cc=peff@peff.net \
    --cc=ps@pks.im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox