Am 23.06.20 um 19:47 schrieb SZEDER Gábor via GitGitGadget:
> From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= <szeder.dev@xxxxxxxxx>
>
> 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.
>
> Signed-off-by: SZEDER Gábor <szeder.dev@xxxxxxxxx>
> Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
> ---
> commit-graph.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/commit-graph.c b/commit-graph.c
> index 086fc2d070..1de6800d74 100644
> --- a/commit-graph.c
> +++ b/commit-graph.c
> @@ -1683,12 +1683,21 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx)
> num_chunks * ctx->commits.nr);
> }
>
> + chunk_offset = f->total + f->offset;
> for (i = 0; i < num_chunks; i++) {
> + uint64_t end_offset;
> +
Hmm, the added code looks complicated because it keeps state outside the
loop, but it could be replace by this:
uint64_t start_offset = f->total + f->offset;
> if (chunks[i].write_fn(f, ctx)) {
> error(_("failed writing chunk with id %"PRIx32""),
> chunks[i].id);
> return -1;
> }
> +
> + end_offset = f->total + f->offset;
> + if (end_offset - chunk_offset != chunks[i].size)
> + BUG("expected to write %"PRId64" bytes to chunk %"PRIx32", but wrote %"PRId64" instead",
> + chunks[i].size, chunks[i].id, end_offset - chunk_offset);
> + chunk_offset = end_offset;
... and that:
if (f->total + f->offset != start_offset + chunks[i].size)
BUG(...);
> }
>
> stop_progress(&ctx->progress);
>
