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> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Derrick Stolee committed
Sep 5, 2025 at 19:26 UTC
1f2bc6be1dbbda5bc40a23bd58a60bbee9de7401
1 file changed
+18
-17
midx-write.c
+18
-17
@@ -1,5 +1,3 @@
1
-#define DISABLE_SIGN_COMPARE_WARNINGS
2
-
1
#include "git-compat-util.h"
2
#include "abspath.h"
3
#include "config.h"
@@ -845,7 +843,7 @@ static int write_midx_bitmap(struct write_midx_context *ctx,
843
uint32_t commits_nr,
844
unsigned flags)
845
{
848
- int ret, i;
846
+ int ret;
847
uint16_t options = 0;
848
struct bitmap_writer writer;
849
struct pack_idx_entry **index;
@@ -873,7 +871,7 @@ static int write_midx_bitmap(struct write_midx_context *ctx,
871
* this order).
872
*/
873
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++)
875
index[i] = &pdata->objects[i].idx;
876
877
bitmap_writer_init(&writer, ctx->repo, pdata,
@@ -894,7 +892,7 @@ static int write_midx_bitmap(struct write_midx_context *ctx,
892
* happens between bitmap_writer_build_type_index() and
893
* bitmap_writer_finish().
894
*/
897
- for (i = 0; i < pdata->nr_objects; i++)
895
+ for (uint32_t i = 0; i < pdata->nr_objects; i++)
896
index[ctx->pack_order[i]] = &pdata->objects[i].idx;
897
898
bitmap_writer_select_commits(&writer, commits, commits_nr);
@@ -1056,7 +1054,7 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
1054
{
1055
struct strbuf midx_name = STRBUF_INIT;
1056
unsigned char midx_hash[GIT_MAX_RAWSZ];
1059
- uint32_t i, start_pack;
1057
+ uint32_t start_pack;
1058
struct hashfile *f = NULL;
1059
struct lock_file lk;
1060
struct tempfile *incr;
@@ -1172,7 +1170,7 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
1170
if (preferred_pack_name) {
1171
ctx.preferred_pack_idx = NO_PREFERRED_PACK;
1172
1175
- for (i = 0; i < ctx.nr; i++) {
1173
+ for (size_t i = 0; i < ctx.nr; i++) {
1174
if (!cmp_idx_or_pack_name(preferred_pack_name,
1175
ctx.info[i].pack_name)) {
1176
ctx.preferred_pack_idx = i;
@@ -1204,7 +1202,7 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
1202
* pack-order has all of its objects selected from that pack
1203
* (and not another pack containing a duplicate)
1204
*/
1207
- for (i = 1; i < ctx.nr; i++) {
1205
+ for (size_t i = 1; i < ctx.nr; i++) {
1206
struct packed_git *p = ctx.info[i].p;
1207
1208
if (!oldest->num_objects || p->mtime < oldest->mtime) {
@@ -1249,7 +1247,7 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
1247
compute_sorted_entries(&ctx, start_pack);
1248
1249
ctx.large_offsets_needed = 0;
1252
- for (i = 0; i < ctx.entries_nr; i++) {
1250
+ for (size_t i = 0; i < ctx.entries_nr; i++) {
1251
if (ctx.entries[i].offset > 0x7fffffff)
1252
ctx.num_large_offsets++;
1253
if (ctx.entries[i].offset > 0xffffffff)
@@ -1259,10 +1257,10 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
1257
QSORT(ctx.info, ctx.nr, pack_info_compare);
1258
1259
if (packs_to_drop && packs_to_drop->nr) {
1262
- int drop_index = 0;
1260
+ size_t drop_index = 0;
1261
int missing_drops = 0;
1262
1265
- for (i = 0; i < ctx.nr && drop_index < packs_to_drop->nr; i++) {
1263
+ for (size_t i = 0; i < ctx.nr && drop_index < packs_to_drop->nr; i++) {
1264
int cmp = strcmp(ctx.info[i].pack_name,
1265
packs_to_drop->items[drop_index].string);
1266
@@ -1293,7 +1291,7 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
1291
* pack_perm[old_id] = new_id
1292
*/
1293
ALLOC_ARRAY(ctx.pack_perm, ctx.nr);
1296
- for (i = 0; i < ctx.nr; i++) {
1294
+ for (size_t i = 0; i < ctx.nr; i++) {
1295
if (ctx.info[i].expired) {
1296
dropped_packs++;
1297
ctx.pack_perm[ctx.info[i].orig_pack_int_id] = PACK_EXPIRED;
@@ -1302,7 +1300,7 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
1300
}
1301
}
1302
1305
- for (i = 0; i < ctx.nr; i++) {
1303
+ for (size_t i = 0; i < ctx.nr; i++) {
1304
if (ctx.info[i].expired)
1305
continue;
1306
pack_name_concat_len += strlen(ctx.info[i].pack_name) + 1;
@@ -1448,6 +1446,9 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
1446
* have been freed in the previous if block.
1447
*/
1448
1449
+ if (ctx.num_multi_pack_indexes_before == UINT32_MAX)
1450
+ die(_("too many multi-pack-indexes"));
1451
+
1452
CALLOC_ARRAY(keep_hashes, ctx.num_multi_pack_indexes_before + 1);
1453
1454
if (ctx.incremental) {
@@ -1480,7 +1481,7 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
1481
keep_hashes[ctx.num_multi_pack_indexes_before] =
1482
xstrdup(hash_to_hex_algop(midx_hash, r->hash_algo));
1483
1483
- for (i = 0; i < ctx.num_multi_pack_indexes_before; i++) {
1484
+ for (uint32_t i = 0; i < ctx.num_multi_pack_indexes_before; i++) {
1485
uint32_t j = ctx.num_multi_pack_indexes_before - i - 1;
1486
1487
keep_hashes[j] = xstrdup(hash_to_hex_algop(get_midx_checksum(m),
@@ -1488,7 +1489,7 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
1489
m = m->base_midx;
1490
}
1491
1491
- for (i = 0; i < ctx.num_multi_pack_indexes_before + 1; i++)
1492
+ for (uint32_t i = 0; i <= ctx.num_multi_pack_indexes_before; i++)
1493
fprintf(get_lock_file_fp(&lk), "%s\n", keep_hashes[i]);
1494
} else {
1495
keep_hashes[ctx.num_multi_pack_indexes_before] =
@@ -1506,7 +1507,7 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
1507
ctx.incremental);
1508
1509
cleanup:
1509
- for (i = 0; i < ctx.nr; i++) {
1510
+ for (size_t i = 0; i < ctx.nr; i++) {
1511
if (ctx.info[i].p) {
1512
close_pack(ctx.info[i].p);
1513
free(ctx.info[i].p);
@@ -1519,7 +1520,7 @@ cleanup:
1520
free(ctx.pack_perm);
1521
free(ctx.pack_order);
1522
if (keep_hashes) {
1522
- for (i = 0; i < ctx.num_multi_pack_indexes_before + 1; i++)
1523
+ for (uint32_t i = 0; i <= ctx.num_multi_pack_indexes_before; i++)
1524
free((char *)keep_hashes[i]);
1525
free(keep_hashes);
1526
}