@matheusbsbs / matheus-bagliano / commits / 0ccd713cb6

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 <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Derrick Stolee committed Feb 18, 2021 at 14:07 UTC 0ccd713cb6c4dafd36bdf85384becbb9b38504ba

1 file changed +27 -36

midx.c

+27 -36

@@ -650,7 +650,7 @@ static struct pack_midx_entry *get_sorted_entries(struct multi_pack_index *m,

650 return deduplicated_entries;

651 }

652

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)

654 {

655 struct write_midx_context *ctx = data;

656 uint32_t i;

@@ -678,14 +678,13 @@ static size_t write_midx_pack_names(struct hashfile *f, void *data)

678 if (i < MIDX_CHUNK_ALIGNMENT) {

679 memset(padding, 0, sizeof(padding));

680 hashwrite(f, padding, i);

681 - written += i;

681 }

682

684 - return written;

683 + return 0;

684 }

685

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)

688 {

689 struct write_midx_context *ctx = data;

690 struct pack_midx_entry *list = ctx->entries;

@@ -710,17 +709,16 @@ static size_t write_midx_oid_fanout(struct hashfile *f,

709 list = next;

710 }

711

713 - return MIDX_CHUNK_FANOUT_SIZE;

712 + return 0;

713 }

714

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)

717 {

718 struct write_midx_context *ctx = data;

719 unsigned char hash_len = the_hash_algo->rawsz;

720 struct pack_midx_entry *list = ctx->entries;

721 uint32_t i;

723 - size_t written = 0;

722

723 for (i = 0; i < ctx->entries_nr; i++) {

724 struct pack_midx_entry *obj = list++;

@@ -734,19 +732,17 @@ static size_t write_midx_oid_lookup(struct hashfile *f,

732 }

733

734 hashwrite(f, obj->oid.hash, (int)hash_len);

737 - written += hash_len;

735 }

736

740 - return written;

737 + return 0;

738 }

739

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)

742 {

743 struct write_midx_context *ctx = data;

744 struct pack_midx_entry *list = ctx->entries;

745 uint32_t i, nr_large_offset = 0;

749 - size_t written = 0;

746

747 for (i = 0; i < ctx->entries_nr; i++) {

748 struct pack_midx_entry *obj = list++;

@@ -766,20 +762,17 @@ static size_t write_midx_object_offsets(struct hashfile *f,

762 obj->offset);

763 else

764 hashwrite_be32(f, (uint32_t)obj->offset);

769 -

770 - written += MIDX_CHUNK_OFFSET_WIDTH;

765 }

766

773 - return written;

767 + return 0;

768 }

769

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)

772 {

773 struct write_midx_context *ctx = data;

774 struct pack_midx_entry *list = ctx->entries;

775 struct pack_midx_entry *end = ctx->entries + ctx->entries_nr;

782 - size_t written = 0;

776 uint32_t nr_large_offset = ctx->num_large_offsets;

777

778 while (nr_large_offset) {

@@ -795,12 +788,12 @@ static size_t write_midx_large_offsets(struct hashfile *f,

788 if (!(offset >> 31))

789 continue;

790

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

791 + hashwrite_be64(f, offset);

792

793 nr_large_offset--;

794 }

795

803 - return written;

796 + return 0;

797 }

798

799 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 *

805 struct hashfile *f = NULL;

806 struct lock_file lk;

807 struct write_midx_context ctx = { 0 };

815 - uint64_t written = 0;

808 + uint64_t header_size = 0;

809 uint32_t chunk_ids[MIDX_MAX_CHUNKS + 1];

810 uint64_t chunk_offsets[MIDX_MAX_CHUNKS + 1];

811 struct progress *progress = NULL;

@@ -940,10 +933,10 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *

933 goto cleanup;

934 }

935

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);

937

938 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;

940

941 cur_chunk++;

942 chunk_ids[cur_chunk] = MIDX_CHUNKID_OIDFANOUT;

@@ -981,39 +974,37 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *

974

975 hashwrite_be32(f, chunk_ids[i]);

976 hashwrite_be64(f, chunk_offsets[i]);

984 -

985 - written += MIDX_CHUNKLOOKUP_WIDTH;

977 }

978

979 if (flags & MIDX_PROGRESS)

980 progress = start_delayed_progress(_("Writing chunks to multi-pack-index"),

981 num_chunks);

982 for (i = 0; i < num_chunks; i++) {

992 - if (written != chunk_offsets[i])

983 + if (f->total + f->offset != chunk_offsets[i])

984 BUG("incorrect chunk offset (%"PRIu64" != %"PRIu64") for chunk id %"PRIx32,

985 chunk_offsets[i],

995 - written,

986 + f->total + f->offset,

987 chunk_ids[i]);

988

989 switch (chunk_ids[i]) {

990 case MIDX_CHUNKID_PACKNAMES:

1000 - written += write_midx_pack_names(f, &ctx);

991 + write_midx_pack_names(f, &ctx);

992 break;

993

994 case MIDX_CHUNKID_OIDFANOUT:

1004 - written += write_midx_oid_fanout(f, &ctx);

995 + write_midx_oid_fanout(f, &ctx);

996 break;

997

998 case MIDX_CHUNKID_OIDLOOKUP:

1008 - written += write_midx_oid_lookup(f, &ctx);

999 + write_midx_oid_lookup(f, &ctx);

1000 break;

1001

1002 case MIDX_CHUNKID_OBJECTOFFSETS:

1012 - written += write_midx_object_offsets(f, &ctx);

1003 + write_midx_object_offsets(f, &ctx);

1004 break;

1005

1006 case MIDX_CHUNKID_LARGEOFFSETS:

1016 - written += write_midx_large_offsets(f, &ctx);

1007 + write_midx_large_offsets(f, &ctx);

1008 break;

1009

1010 default:

@@ -1025,9 +1016,9 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *

1016 }

1017 stop_progress(&progress);

1018

1028 - if (written != chunk_offsets[num_chunks])

1019 + if (hashfile_total(f) != chunk_offsets[num_chunks])

1020 BUG("incorrect final offset %"PRIu64" != %"PRIu64,

1030 - written,

1021 + hashfile_total(f),

1022 chunk_offsets[num_chunks]);

1023

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