@matheusbsbs / matheus-bagliano / commits / 2dd4fed927
Fork 0

commit-graph: check chunk sizes after writing

In my experience while experimenting with new commit-graph chunks, early versions of the corresponding new write_commit_graph_my_chunk() functions are, sadly but not surprisingly, often buggy, and write more or less data than they are supposed to, especially if the chunk size is not directly proportional to the number of commits. This then causes all kinds of issues when reading such a bogus commit-graph file, raising the question of whether the writing or the reading part happens to be buggy this time. Let's catch such issues early, already when writing the commit-graph file, and check that each write_graph_chunk_*() function wrote the amount of data that it was expected to, and what has been encoded in the Chunk Lookup table. Now that all commit-graph chunks are written in a loop we can do this check in a single place for all chunks, and any chunks added in the future will get checked as well. Helped-by: René Scharfe <l.s.r@web.de> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

SZEDER Gábor committed Jul 1, 2020 at 13:27 UTC 2dd4fed92715fddcefc0223b79e51af2798e921e
1 file changed +7
commit-graph.c
+7
@@ -1693,8 +1693,15 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx)
1693 }
1694
1695 for (i = 0; i < num_chunks; i++) {
1696 + uint64_t start_offset = f->total + f->offset;
1697 +
1698 if (chunks[i].write_fn(f, ctx))
1699 return -1;
1700 +
1701 + if (f->total + f->offset != start_offset + chunks[i].size)
1702 + BUG("expected to write %"PRId64" bytes to chunk %"PRIx32", but wrote %"PRId64" instead",
1703 + chunks[i].size, chunks[i].id,
1704 + f->total + f->offset - start_offset);
1705 }
1706
1707 stop_progress(&ctx->progress);