Skip to content

Commit 0ccd713

Browse files
derrickstoleegitster
authored andcommitted
midx: return success/failure in chunk write methods
Historically, the chunk-writing methods in midx.c have returned the amount of data written so the writer method could compare this with the table of contents. This presents with some interesting issues: 1. If a chunk writing method has a bug that miscalculates the written bytes, then we can satisfy the table of contents without actually writing the right amount of data to the hashfile. The commit-graph writing code checks the hashfile struct directly for a more robust verification. 2. There is no way for a chunk writing method to gracefully fail. Returning an int presents an opportunity to fail without a die(). 3. The current pattern doesn't match chunk_write_fn type exactly, so we cannot share code with commit-graph.c For these reasons, convert the midx chunk writer methods to return an 'int'. Since none of them fail at the moment, they all return 0. Signed-off-by: Derrick Stolee <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 980f525 commit 0ccd713

File tree

1 file changed

+27
-36
lines changed

1 file changed

+27
-36
lines changed

midx.c

Lines changed: 27 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -650,7 +650,7 @@ static struct pack_midx_entry *get_sorted_entries(struct multi_pack_index *m,
650650
return deduplicated_entries;
651651
}
652652

653-
static size_t write_midx_pack_names(struct hashfile *f, void *data)
653+
static int write_midx_pack_names(struct hashfile *f, void *data)
654654
{
655655
struct write_midx_context *ctx = data;
656656
uint32_t i;
@@ -678,14 +678,13 @@ static size_t write_midx_pack_names(struct hashfile *f, void *data)
678678
if (i < MIDX_CHUNK_ALIGNMENT) {
679679
memset(padding, 0, sizeof(padding));
680680
hashwrite(f, padding, i);
681-
written += i;
682681
}
683682

684-
return written;
683+
return 0;
685684
}
686685

687-
static size_t write_midx_oid_fanout(struct hashfile *f,
688-
void *data)
686+
static int write_midx_oid_fanout(struct hashfile *f,
687+
void *data)
689688
{
690689
struct write_midx_context *ctx = data;
691690
struct pack_midx_entry *list = ctx->entries;
@@ -710,17 +709,16 @@ static size_t write_midx_oid_fanout(struct hashfile *f,
710709
list = next;
711710
}
712711

713-
return MIDX_CHUNK_FANOUT_SIZE;
712+
return 0;
714713
}
715714

716-
static size_t write_midx_oid_lookup(struct hashfile *f,
717-
void *data)
715+
static int write_midx_oid_lookup(struct hashfile *f,
716+
void *data)
718717
{
719718
struct write_midx_context *ctx = data;
720719
unsigned char hash_len = the_hash_algo->rawsz;
721720
struct pack_midx_entry *list = ctx->entries;
722721
uint32_t i;
723-
size_t written = 0;
724722

725723
for (i = 0; i < ctx->entries_nr; i++) {
726724
struct pack_midx_entry *obj = list++;
@@ -734,19 +732,17 @@ static size_t write_midx_oid_lookup(struct hashfile *f,
734732
}
735733

736734
hashwrite(f, obj->oid.hash, (int)hash_len);
737-
written += hash_len;
738735
}
739736

740-
return written;
737+
return 0;
741738
}
742739

743-
static size_t write_midx_object_offsets(struct hashfile *f,
744-
void *data)
740+
static int write_midx_object_offsets(struct hashfile *f,
741+
void *data)
745742
{
746743
struct write_midx_context *ctx = data;
747744
struct pack_midx_entry *list = ctx->entries;
748745
uint32_t i, nr_large_offset = 0;
749-
size_t written = 0;
750746

751747
for (i = 0; i < ctx->entries_nr; i++) {
752748
struct pack_midx_entry *obj = list++;
@@ -766,20 +762,17 @@ static size_t write_midx_object_offsets(struct hashfile *f,
766762
obj->offset);
767763
else
768764
hashwrite_be32(f, (uint32_t)obj->offset);
769-
770-
written += MIDX_CHUNK_OFFSET_WIDTH;
771765
}
772766

773-
return written;
767+
return 0;
774768
}
775769

776-
static size_t write_midx_large_offsets(struct hashfile *f,
777-
void *data)
770+
static int write_midx_large_offsets(struct hashfile *f,
771+
void *data)
778772
{
779773
struct write_midx_context *ctx = data;
780774
struct pack_midx_entry *list = ctx->entries;
781775
struct pack_midx_entry *end = ctx->entries + ctx->entries_nr;
782-
size_t written = 0;
783776
uint32_t nr_large_offset = ctx->num_large_offsets;
784777

785778
while (nr_large_offset) {
@@ -795,12 +788,12 @@ static size_t write_midx_large_offsets(struct hashfile *f,
795788
if (!(offset >> 31))
796789
continue;
797790

798-
written += hashwrite_be64(f, offset);
791+
hashwrite_be64(f, offset);
799792

800793
nr_large_offset--;
801794
}
802795

803-
return written;
796+
return 0;
804797
}
805798

806799
static int write_midx_internal(const char *object_dir, struct multi_pack_index *m,
@@ -812,7 +805,7 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *
812805
struct hashfile *f = NULL;
813806
struct lock_file lk;
814807
struct write_midx_context ctx = { 0 };
815-
uint64_t written = 0;
808+
uint64_t header_size = 0;
816809
uint32_t chunk_ids[MIDX_MAX_CHUNKS + 1];
817810
uint64_t chunk_offsets[MIDX_MAX_CHUNKS + 1];
818811
struct progress *progress = NULL;
@@ -940,10 +933,10 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *
940933
goto cleanup;
941934
}
942935

943-
written = write_midx_header(f, num_chunks, ctx.nr - dropped_packs);
936+
header_size = write_midx_header(f, num_chunks, ctx.nr - dropped_packs);
944937

945938
chunk_ids[cur_chunk] = MIDX_CHUNKID_PACKNAMES;
946-
chunk_offsets[cur_chunk] = written + (num_chunks + 1) * MIDX_CHUNKLOOKUP_WIDTH;
939+
chunk_offsets[cur_chunk] = header_size + (num_chunks + 1) * MIDX_CHUNKLOOKUP_WIDTH;
947940

948941
cur_chunk++;
949942
chunk_ids[cur_chunk] = MIDX_CHUNKID_OIDFANOUT;
@@ -981,39 +974,37 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *
981974

982975
hashwrite_be32(f, chunk_ids[i]);
983976
hashwrite_be64(f, chunk_offsets[i]);
984-
985-
written += MIDX_CHUNKLOOKUP_WIDTH;
986977
}
987978

988979
if (flags & MIDX_PROGRESS)
989980
progress = start_delayed_progress(_("Writing chunks to multi-pack-index"),
990981
num_chunks);
991982
for (i = 0; i < num_chunks; i++) {
992-
if (written != chunk_offsets[i])
983+
if (f->total + f->offset != chunk_offsets[i])
993984
BUG("incorrect chunk offset (%"PRIu64" != %"PRIu64") for chunk id %"PRIx32,
994985
chunk_offsets[i],
995-
written,
986+
f->total + f->offset,
996987
chunk_ids[i]);
997988

998989
switch (chunk_ids[i]) {
999990
case MIDX_CHUNKID_PACKNAMES:
1000-
written += write_midx_pack_names(f, &ctx);
991+
write_midx_pack_names(f, &ctx);
1001992
break;
1002993

1003994
case MIDX_CHUNKID_OIDFANOUT:
1004-
written += write_midx_oid_fanout(f, &ctx);
995+
write_midx_oid_fanout(f, &ctx);
1005996
break;
1006997

1007998
case MIDX_CHUNKID_OIDLOOKUP:
1008-
written += write_midx_oid_lookup(f, &ctx);
999+
write_midx_oid_lookup(f, &ctx);
10091000
break;
10101001

10111002
case MIDX_CHUNKID_OBJECTOFFSETS:
1012-
written += write_midx_object_offsets(f, &ctx);
1003+
write_midx_object_offsets(f, &ctx);
10131004
break;
10141005

10151006
case MIDX_CHUNKID_LARGEOFFSETS:
1016-
written += write_midx_large_offsets(f, &ctx);
1007+
write_midx_large_offsets(f, &ctx);
10171008
break;
10181009

10191010
default:
@@ -1025,9 +1016,9 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *
10251016
}
10261017
stop_progress(&progress);
10271018

1028-
if (written != chunk_offsets[num_chunks])
1019+
if (hashfile_total(f) != chunk_offsets[num_chunks])
10291020
BUG("incorrect final offset %"PRIu64" != %"PRIu64,
1030-
written,
1021+
hashfile_total(f),
10311022
chunk_offsets[num_chunks]);
10321023

10331024
finalize_hashfile(f, NULL, CSUM_FSYNC | CSUM_HASH_IN_STREAM);

0 commit comments

Comments
 (0)