Skip to content

Commit 35302f5

Browse files
committed
midx-write: reenable signed comparison errors
Remove the remaining signed comparison warnings in midx-write.c so that they can be enforced as errors in the future. After the previous change, the remaining errors are due to iterator variables named 'i'. The strategy here involves defining the variable within the for loop syntax to make sure we use the appropriate bitness for the loop sentinel. This matters in at least one method where the variable was compared to uint32_t in some loops and size_t in others. While adjusting these loops, there were some where the loop boundary was checking against a uint32_t value _plus one_. These were replaced with non-strict comparisons, but also the value is checked to not be UINT32_MAX. Since the value is the number of incremental multi-pack- indexes, this is not a meaningful restriction. The new die() is about defensive programming more than it being realistically possible. Signed-off-by: Derrick Stolee <stolee@gmail.com>
1 parent 2290e27 commit 35302f5

1 file changed

Lines changed: 18 additions & 17 deletions

File tree

midx-write.c

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
#define DISABLE_SIGN_COMPARE_WARNINGS
2-
31
#include "git-compat-util.h"
42
#include "abspath.h"
53
#include "config.h"
@@ -845,7 +843,7 @@ static int write_midx_bitmap(struct write_midx_context *ctx,
845843
uint32_t commits_nr,
846844
unsigned flags)
847845
{
848-
int ret, i;
846+
int ret;
849847
uint16_t options = 0;
850848
struct bitmap_writer writer;
851849
struct pack_idx_entry **index;
@@ -873,7 +871,7 @@ static int write_midx_bitmap(struct write_midx_context *ctx,
873871
* this order).
874872
*/
875873
ALLOC_ARRAY(index, pdata->nr_objects);
876-
for (i = 0; i < pdata->nr_objects; i++)
874+
for (uint32_t i = 0; i < pdata->nr_objects; i++)
877875
index[i] = &pdata->objects[i].idx;
878876

879877
bitmap_writer_init(&writer, ctx->repo, pdata,
@@ -894,7 +892,7 @@ static int write_midx_bitmap(struct write_midx_context *ctx,
894892
* happens between bitmap_writer_build_type_index() and
895893
* bitmap_writer_finish().
896894
*/
897-
for (i = 0; i < pdata->nr_objects; i++)
895+
for (uint32_t i = 0; i < pdata->nr_objects; i++)
898896
index[ctx->pack_order[i]] = &pdata->objects[i].idx;
899897

900898
bitmap_writer_select_commits(&writer, commits, commits_nr);
@@ -1038,7 +1036,7 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
10381036
{
10391037
struct strbuf midx_name = STRBUF_INIT;
10401038
unsigned char midx_hash[GIT_MAX_RAWSZ];
1041-
uint32_t i, start_pack;
1039+
uint32_t start_pack;
10421040
struct hashfile *f = NULL;
10431041
struct lock_file lk;
10441042
struct tempfile *incr;
@@ -1154,7 +1152,7 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
11541152
if (preferred_pack_name) {
11551153
ctx.preferred_pack_idx = NO_PREFERRED_PACK;
11561154

1157-
for (i = 0; i < ctx.nr; i++) {
1155+
for (size_t i = 0; i < ctx.nr; i++) {
11581156
if (!cmp_idx_or_pack_name(preferred_pack_name,
11591157
ctx.info[i].pack_name)) {
11601158
ctx.preferred_pack_idx = i;
@@ -1186,7 +1184,7 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
11861184
* pack-order has all of its objects selected from that pack
11871185
* (and not another pack containing a duplicate)
11881186
*/
1189-
for (i = 1; i < ctx.nr; i++) {
1187+
for (size_t i = 1; i < ctx.nr; i++) {
11901188
struct packed_git *p = ctx.info[i].p;
11911189

11921190
if (!oldest->num_objects || p->mtime < oldest->mtime) {
@@ -1231,7 +1229,7 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
12311229
compute_sorted_entries(&ctx, start_pack);
12321230

12331231
ctx.large_offsets_needed = 0;
1234-
for (i = 0; i < ctx.entries_nr; i++) {
1232+
for (size_t i = 0; i < ctx.entries_nr; i++) {
12351233
if (ctx.entries[i].offset > 0x7fffffff)
12361234
ctx.num_large_offsets++;
12371235
if (ctx.entries[i].offset > 0xffffffff)
@@ -1241,10 +1239,10 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
12411239
QSORT(ctx.info, ctx.nr, pack_info_compare);
12421240

12431241
if (packs_to_drop && packs_to_drop->nr) {
1244-
int drop_index = 0;
1242+
size_t drop_index = 0;
12451243
int missing_drops = 0;
12461244

1247-
for (i = 0; i < ctx.nr && drop_index < packs_to_drop->nr; i++) {
1245+
for (size_t i = 0; i < ctx.nr && drop_index < packs_to_drop->nr; i++) {
12481246
int cmp = strcmp(ctx.info[i].pack_name,
12491247
packs_to_drop->items[drop_index].string);
12501248

@@ -1275,7 +1273,7 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
12751273
* pack_perm[old_id] = new_id
12761274
*/
12771275
ALLOC_ARRAY(ctx.pack_perm, ctx.nr);
1278-
for (i = 0; i < ctx.nr; i++) {
1276+
for (size_t i = 0; i < ctx.nr; i++) {
12791277
if (ctx.info[i].expired) {
12801278
dropped_packs++;
12811279
ctx.pack_perm[ctx.info[i].orig_pack_int_id] = PACK_EXPIRED;
@@ -1284,7 +1282,7 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
12841282
}
12851283
}
12861284

1287-
for (i = 0; i < ctx.nr; i++) {
1285+
for (size_t i = 0; i < ctx.nr; i++) {
12881286
if (ctx.info[i].expired)
12891287
continue;
12901288
pack_name_concat_len += strlen(ctx.info[i].pack_name) + 1;
@@ -1430,6 +1428,9 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
14301428
* have been freed in the previous if block.
14311429
*/
14321430

1431+
if (ctx.num_multi_pack_indexes_before == UINT32_MAX)
1432+
die("too many multi-pack-indexes");
1433+
14331434
CALLOC_ARRAY(keep_hashes, ctx.num_multi_pack_indexes_before + 1);
14341435

14351436
if (ctx.incremental) {
@@ -1462,15 +1463,15 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
14621463
keep_hashes[ctx.num_multi_pack_indexes_before] =
14631464
xstrdup(hash_to_hex_algop(midx_hash, r->hash_algo));
14641465

1465-
for (i = 0; i < ctx.num_multi_pack_indexes_before; i++) {
1466+
for (uint32_t i = 0; i < ctx.num_multi_pack_indexes_before; i++) {
14661467
uint32_t j = ctx.num_multi_pack_indexes_before - i - 1;
14671468

14681469
keep_hashes[j] = xstrdup(hash_to_hex_algop(get_midx_checksum(m),
14691470
r->hash_algo));
14701471
m = m->base_midx;
14711472
}
14721473

1473-
for (i = 0; i < ctx.num_multi_pack_indexes_before + 1; i++)
1474+
for (uint32_t i = 0; i <= ctx.num_multi_pack_indexes_before; i++)
14741475
fprintf(get_lock_file_fp(&lk), "%s\n", keep_hashes[i]);
14751476
} else {
14761477
keep_hashes[ctx.num_multi_pack_indexes_before] =
@@ -1488,7 +1489,7 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
14881489
ctx.incremental);
14891490

14901491
cleanup:
1491-
for (i = 0; i < ctx.nr; i++) {
1492+
for (size_t i = 0; i < ctx.nr; i++) {
14921493
if (ctx.info[i].p) {
14931494
close_pack(ctx.info[i].p);
14941495
free(ctx.info[i].p);
@@ -1501,7 +1502,7 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
15011502
free(ctx.pack_perm);
15021503
free(ctx.pack_order);
15031504
if (keep_hashes) {
1504-
for (i = 0; i < ctx.num_multi_pack_indexes_before + 1; i++)
1505+
for (uint32_t i = 0; i <= ctx.num_multi_pack_indexes_before; i++)
15051506
free((char *)keep_hashes[i]);
15061507
free(keep_hashes);
15071508
}

0 commit comments

Comments
 (0)