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>

diff --git a/midx-write.c b/midx-write.c
index e8864386d8d84ea4b30151c777d28a0483682620..fd1803c3ec72d8e2a9413664c4c878d68bdd287c 100644 (file)
--- 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);
@@ -1058,7 +1060,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;
@@ -1166,7 +1170,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,
@@ -1176,12 +1180,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;
 
                /*
@@ -1217,17 +1221,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))