From: "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail•com>
To: git@vger•kernel.org
Cc: gitster@pobox•com, me@ttaylorr•com,
Derrick Stolee <stolee@gmail•com>,
Derrick Stolee <stolee@gmail•com>
Subject: [PATCH v2 4/6] midx-write: use uint32_t for preferred_pack_idx
Date: Sat, 30 Aug 2025 21:23:25 +0000 [thread overview]
Message-ID: <2290e27ded03866cb816be5fb6e08b83a75a6f07.1756589007.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1965.v2.git.1756589007.gitgitgadget@gmail.com>
From: Derrick Stolee <stolee@gmail•com>
midx-write.c has the DISABLE_SIGN_COMPARE_WARNINGS macro defined for a
few reasons, but the biggest one is the use of a signed
preferred_pack_idx member inside the write_midx_context struct. The code
currently uses -1 to indicate an unset preferred pack but pack int ids
are normally handled as uint32_t. There are also a few loops that search
for the preferred pack by name and those iterators will need updates to
uint32_t in the next change.
For now, replace the use of -1 with a 'NO_PREFERRED_PACK' macro and an
equality check. The macro stores the max value of a uint32_t, so we
cannot store a preferred pack that appears last in a list of 2^32 total
packs, but that's expected to be unreasonable already. This improves the
range from 2^31 already.
There are some careful things to worry about with initializing the
preferred pack in the struct and using that value when searching for a
preferred pack that was already incorrect but accidentally working when
the index was initialized to zero.
Signed-off-by: Derrick Stolee <stolee@gmail•com>
---
midx-write.c | 26 +++++++++++++++-----------
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/midx-write.c b/midx-write.c
index cb0211289d..1822268ce2 100644
--- a/midx-write.c
+++ b/midx-write.c
@@ -24,6 +24,7 @@
#define BITMAP_POS_UNKNOWN (~((uint32_t)0))
#define MIDX_CHUNK_FANOUT_SIZE (sizeof(uint32_t) * 256)
#define MIDX_CHUNK_LARGE_OFFSET_WIDTH (sizeof(uint64_t))
+#define NO_PREFERRED_PACK (~((uint32_t)0))
extern int midx_checksum_valid(struct multi_pack_index *m);
extern void clear_midx_files_ext(const char *object_dir, const char *ext,
@@ -104,7 +105,7 @@ struct write_midx_context {
unsigned large_offsets_needed:1;
uint32_t num_large_offsets;
- int preferred_pack_idx;
+ uint32_t preferred_pack_idx;
int incremental;
uint32_t num_multi_pack_indexes_before;
@@ -260,7 +261,7 @@ static void midx_fanout_sort(struct midx_fanout *fanout)
static void midx_fanout_add_midx_fanout(struct midx_fanout *fanout,
struct multi_pack_index *m,
uint32_t cur_fanout,
- int preferred_pack)
+ uint32_t preferred_pack)
{
uint32_t start = m->num_objects_in_base, end;
uint32_t cur_object;
@@ -274,7 +275,7 @@ static void midx_fanout_add_midx_fanout(struct midx_fanout *fanout,
end = m->num_objects_in_base + ntohl(m->chunk_oid_fanout[cur_fanout]);
for (cur_object = start; cur_object < end; cur_object++) {
- if ((preferred_pack > -1) &&
+ if ((preferred_pack != NO_PREFERRED_PACK) &&
(preferred_pack == nth_midxed_pack_int_id(m, cur_object))) {
/*
* Objects from preferred packs are added
@@ -364,7 +365,8 @@ static void compute_sorted_entries(struct write_midx_context *ctx,
preferred, cur_fanout);
}
- if (-1 < ctx->preferred_pack_idx && ctx->preferred_pack_idx < start_pack)
+ if (ctx->preferred_pack_idx != NO_PREFERRED_PACK &&
+ ctx->preferred_pack_idx < start_pack)
midx_fanout_add_pack_fanout(&fanout, ctx->info,
ctx->preferred_pack_idx, 1,
cur_fanout);
@@ -1040,7 +1042,9 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
struct hashfile *f = NULL;
struct lock_file lk;
struct tempfile *incr;
- struct write_midx_context ctx = { 0 };
+ struct write_midx_context ctx = {
+ .preferred_pack_idx = NO_PREFERRED_PACK,
+ };
int bitmapped_packs_concat_len = 0;
int pack_name_concat_len = 0;
int dropped_packs = 0;
@@ -1148,7 +1152,7 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
goto cleanup; /* nothing to do */
if (preferred_pack_name) {
- ctx.preferred_pack_idx = -1;
+ ctx.preferred_pack_idx = NO_PREFERRED_PACK;
for (i = 0; i < ctx.nr; i++) {
if (!cmp_idx_or_pack_name(preferred_pack_name,
@@ -1158,12 +1162,12 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
}
}
- if (ctx.preferred_pack_idx == -1)
+ if (ctx.preferred_pack_idx == NO_PREFERRED_PACK)
warning(_("unknown preferred pack: '%s'"),
preferred_pack_name);
} else if (ctx.nr &&
(flags & (MIDX_WRITE_REV_INDEX | MIDX_WRITE_BITMAP))) {
- struct packed_git *oldest = ctx.info[ctx.preferred_pack_idx].p;
+ struct packed_git *oldest = ctx.info[0].p;
ctx.preferred_pack_idx = 0;
/*
@@ -1199,17 +1203,17 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
* objects to resolve, so the preferred value doesn't
* matter.
*/
- ctx.preferred_pack_idx = -1;
+ ctx.preferred_pack_idx = NO_PREFERRED_PACK;
}
} else {
/*
* otherwise don't mark any pack as preferred to avoid
* interfering with expiration logic below
*/
- ctx.preferred_pack_idx = -1;
+ ctx.preferred_pack_idx = NO_PREFERRED_PACK;
}
- if (ctx.preferred_pack_idx > -1) {
+ if (ctx.preferred_pack_idx != NO_PREFERRED_PACK) {
struct packed_git *preferred = ctx.info[ctx.preferred_pack_idx].p;
if (open_pack_index(preferred))
--
gitgitgadget
next prev parent reply other threads:[~2025-08-30 21:23 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-08-28 17:39 [PATCH 0/5] midx-write: fix segfault and do several cleanups Derrick Stolee via GitGitGadget
2025-08-28 17:39 ` [PATCH 1/5] midx-write: only load initialized packs Derrick Stolee via GitGitGadget
2025-08-28 20:19 ` Junio C Hamano
2025-08-29 1:20 ` Taylor Blau
2025-08-30 14:33 ` Derrick Stolee
2025-08-28 17:39 ` [PATCH 2/5] midx-write: put failing response value back Derrick Stolee via GitGitGadget
2025-08-28 20:45 ` Junio C Hamano
2025-08-29 1:26 ` Taylor Blau
2025-08-28 17:39 ` [PATCH 3/5] midx-write: use cleanup when incremental midx fails Derrick Stolee via GitGitGadget
2025-08-28 20:51 ` Junio C Hamano
2025-08-29 1:29 ` Taylor Blau
2025-08-30 14:44 ` Derrick Stolee
2025-08-28 17:39 ` [PATCH 4/5] midx-write: use uint32_t for preferred_pack_idx Derrick Stolee via GitGitGadget
2025-08-28 20:58 ` Junio C Hamano
2025-08-29 1:35 ` Taylor Blau
2025-08-28 17:39 ` [PATCH 5/5] midx-write: reenable signed comparison errors Derrick Stolee via GitGitGadget
2025-08-28 21:01 ` Junio C Hamano
2025-08-29 1:35 ` Taylor Blau
2025-08-29 1:36 ` [PATCH 0/5] midx-write: fix segfault and do several cleanups Taylor Blau
2025-08-30 21:23 ` [PATCH v2 0/6] " Derrick Stolee via GitGitGadget
2025-08-30 21:23 ` [PATCH v2 1/6] midx-write: only load initialized packs Derrick Stolee via GitGitGadget
2025-09-03 10:14 ` Patrick Steinhardt
2025-09-05 18:58 ` Derrick Stolee
2025-08-30 21:23 ` [PATCH v2 2/6] midx-write: put failing response value back Derrick Stolee via GitGitGadget
2025-09-03 10:15 ` Patrick Steinhardt
2025-09-05 19:03 ` Derrick Stolee
2025-08-30 21:23 ` [PATCH v2 3/6] midx-write: use cleanup when incremental midx fails Derrick Stolee via GitGitGadget
2025-09-03 10:15 ` Patrick Steinhardt
2025-08-30 21:23 ` Derrick Stolee via GitGitGadget [this message]
2025-09-03 10:15 ` [PATCH v2 4/6] midx-write: use uint32_t for preferred_pack_idx Patrick Steinhardt
2025-09-05 19:05 ` Derrick Stolee
2025-08-30 21:23 ` [PATCH v2 5/6] midx-write: reenable signed comparison errors Derrick Stolee via GitGitGadget
2025-09-03 10:15 ` Patrick Steinhardt
2025-08-30 21:23 ` [PATCH v2 6/6] midx-write: simplify error cases Derrick Stolee via GitGitGadget
2025-09-03 10:15 ` Patrick Steinhardt
2025-09-03 18:43 ` Junio C Hamano
2025-09-05 19:26 ` [PATCH v3 0/6] midx-write: fix segfault and do several cleanups Derrick Stolee via GitGitGadget
2025-09-05 19:26 ` [PATCH v3 1/6] midx-write: only load initialized packs Derrick Stolee via GitGitGadget
2025-09-05 19:26 ` [PATCH v3 2/6] midx-write: put failing response value back Derrick Stolee via GitGitGadget
2025-09-05 19:26 ` [PATCH v3 3/6] midx-write: use cleanup when incremental midx fails Derrick Stolee via GitGitGadget
2025-09-05 19:26 ` [PATCH v3 4/6] midx-write: use uint32_t for preferred_pack_idx Derrick Stolee via GitGitGadget
2025-09-05 19:26 ` [PATCH v3 5/6] midx-write: reenable signed comparison errors Derrick Stolee via GitGitGadget
2025-09-05 19:26 ` [PATCH v3 6/6] midx-write: simplify error cases Derrick Stolee via GitGitGadget
2025-09-05 19:38 ` [PATCH v3 0/6] midx-write: fix segfault and do several cleanups Junio C Hamano
2025-09-05 19:57 ` Derrick Stolee
2025-09-11 23:13 ` Taylor Blau
2025-09-11 23:44 ` Junio C Hamano
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=2290e27ded03866cb816be5fb6e08b83a75a6f07.1756589007.git.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=me@ttaylorr$(echo .)com \
--cc=stolee@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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