From: Derrick Stolee <stolee@xxxxxxxxx>
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@xxxxxxxxx>
---
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
