midx-write: use uint32_t for preferred_pack_idx

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. Furthermore, with this change we end up extending the range from 2^31 possible packs to 2^32-1. 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> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Derrick Stolee committed Sep 5, 2025 at 19:26 UTC 68383ac9d4c90c9b3c7591aa0118d72489599b11
1 file changed +15 -11
midx-write.c
+15 -11
@@ -24,6 +24,7 @@
24 #define BITMAP_POS_UNKNOWN (~((uint32_t)0))
25 #define MIDX_CHUNK_FANOUT_SIZE (sizeof(uint32_t) * 256)
26 #define MIDX_CHUNK_LARGE_OFFSET_WIDTH (sizeof(uint64_t))
27 +#define NO_PREFERRED_PACK (~((uint32_t)0))
28
29 extern int midx_checksum_valid(struct multi_pack_index *m);
30 extern void clear_midx_files_ext(const char *object_dir, const char *ext,
@@ -104,7 +105,7 @@ struct write_midx_context {
105 unsigned large_offsets_needed:1;
106 uint32_t num_large_offsets;
107
107 - int preferred_pack_idx;
108 + uint32_t preferred_pack_idx;
109
110 int incremental;
111 uint32_t num_multi_pack_indexes_before;
@@ -260,7 +261,7 @@ static void midx_fanout_sort(struct midx_fanout *fanout)
261 static void midx_fanout_add_midx_fanout(struct midx_fanout *fanout,
262 struct multi_pack_index *m,
263 uint32_t cur_fanout,
263 - int preferred_pack)
264 + uint32_t preferred_pack)
265 {
266 uint32_t start = m->num_objects_in_base, end;
267 uint32_t cur_object;
@@ -274,7 +275,7 @@ static void midx_fanout_add_midx_fanout(struct midx_fanout *fanout,
275 end = m->num_objects_in_base + ntohl(m->chunk_oid_fanout[cur_fanout]);
276
277 for (cur_object = start; cur_object < end; cur_object++) {
277 - if ((preferred_pack > -1) &&
278 + if ((preferred_pack != NO_PREFERRED_PACK) &&
279 (preferred_pack == nth_midxed_pack_int_id(m, cur_object))) {
280 /*
281 * Objects from preferred packs are added
@@ -364,7 +365,8 @@ static void compute_sorted_entries(struct write_midx_context *ctx,
365 preferred, cur_fanout);
366 }
367
367 - if (-1 < ctx->preferred_pack_idx && ctx->preferred_pack_idx < start_pack)
368 + if (ctx->preferred_pack_idx != NO_PREFERRED_PACK &&
369 + ctx->preferred_pack_idx < start_pack)
370 midx_fanout_add_pack_fanout(&fanout, ctx->info,
371 ctx->preferred_pack_idx, 1,
372 cur_fanout);
@@ -1058,7 +1060,9 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
1060 struct hashfile *f = NULL;
1061 struct lock_file lk;
1062 struct tempfile *incr;
1061 - struct write_midx_context ctx = { 0 };
1063 + struct write_midx_context ctx = {
1064 + .preferred_pack_idx = NO_PREFERRED_PACK,
1065 + };
1066 int bitmapped_packs_concat_len = 0;
1067 int pack_name_concat_len = 0;
1068 int dropped_packs = 0;
@@ -1166,7 +1170,7 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
1170 goto cleanup; /* nothing to do */
1171
1172 if (preferred_pack_name) {
1169 - ctx.preferred_pack_idx = -1;
1173 + ctx.preferred_pack_idx = NO_PREFERRED_PACK;
1174
1175 for (i = 0; i < ctx.nr; i++) {
1176 if (!cmp_idx_or_pack_name(preferred_pack_name,
@@ -1176,12 +1180,12 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
1180 }
1181 }
1182
1179 - if (ctx.preferred_pack_idx == -1)
1183 + if (ctx.preferred_pack_idx == NO_PREFERRED_PACK)
1184 warning(_("unknown preferred pack: '%s'"),
1185 preferred_pack_name);
1186 } else if (ctx.nr &&
1187 (flags & (MIDX_WRITE_REV_INDEX | MIDX_WRITE_BITMAP))) {
1184 - struct packed_git *oldest = ctx.info[ctx.preferred_pack_idx].p;
1188 + struct packed_git *oldest = ctx.info[0].p;
1189 ctx.preferred_pack_idx = 0;
1190
1191 /*
@@ -1217,17 +1221,17 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
1221 * objects to resolve, so the preferred value doesn't
1222 * matter.
1223 */
1220 - ctx.preferred_pack_idx = -1;
1224 + ctx.preferred_pack_idx = NO_PREFERRED_PACK;
1225 }
1226 } else {
1227 /*
1228 * otherwise don't mark any pack as preferred to avoid
1229 * interfering with expiration logic below
1230 */
1227 - ctx.preferred_pack_idx = -1;
1231 + ctx.preferred_pack_idx = NO_PREFERRED_PACK;
1232 }
1233
1230 - if (ctx.preferred_pack_idx > -1) {
1234 + if (ctx.preferred_pack_idx != NO_PREFERRED_PACK) {
1235 struct packed_git *preferred = ctx.info[ctx.preferred_pack_idx].p;
1236
1237 if (open_pack_index(preferred))