commit-graph: abort as soon as we see a bogus chunk
commit8bd40ed2aed2baee235299cdbf8482c752f980a3
authorJeff King <peff@peff.net>
Thu, 9 Nov 2023 07:17:11 +0000 (9 02:17 -0500)
committerJunio C Hamano <gitster@pobox.com>
Thu, 9 Nov 2023 10:07:53 +0000 (9 19:07 +0900)
treece82e584ad48f0fca5634de77994b4695a478f85
parent93d29247298e9ae3fbc6dd8e022a6260b568191a
commit-graph: abort as soon as we see a bogus chunk

The code to read commit-graph files tries to read all of the required
chunks, but doesn't abort if we can't find one (or if it's corrupted).
It's only at the end of reading the file that we then do some sanity
checks for NULL entries. But it's preferable to detect the errors and
bail immediately, for a few reasons:

  1. It's less error-prone. It's easy in the reader functions to flag an
     error but still end up setting some struct fields (an error I in
     fact made while working on this patch series).

  2. It's safer. Since verifying some chunks depends on the values of
     other chunks, we may be depending on not-yet-verified data. I don't
     know offhand of any case where this can cause problems, but it's
     one less subtle thing to worry about in the reader code.

  3. It prevents the user from seeing nonsense errors. If we're missing
     an OIDL chunk, then g->num_commits will be zero. And so we may
     complain that the size of our CDAT chunk (which should have a
     fixed-size record for each commit) is wrong unless it's also zero.
     But that's misleading; the problem is the missing OIDL chunk; the
     CDAT one might be fine!

So let's just check the return value from read_chunk(). This is exactly
how the midx chunk-reading code does it.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
commit-graph.c