Re: [RFC PATCH 1/1] commit-graph.c: die on un-parseable commits
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 5, 2019, 22:19 UTC
- Message-ID
- <xmqqsgpao06z.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <20190904030456.GA28836@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 17 quoted lines
> I don't think parse_commit_no_graph() returning 0 assures us that > get_commit_tree() and friends will return non-NULL. > > This is similar to the case discussed recently where a failed parse of a > tag may leave "tag->tagged == NULL" even though "tag->obj.parsed" is > set. > > Here an earlier parsing error could cause (*list)->object.parsed to be > true, but with (*list)->maybe_tree still NULL. Our call to > parse_commit_no_graph() here would silently return "yep, already tried > to parse this", and then we'd still segfault. > ... > And I think there's literally no way for this function to tell the > difference between "no parent" and "there was an earlier error, but we > set the parsed flag anyway and the parent flag is invalid". > > I think that argues against Junio's response in:
Fair enough. Forcing later users to reattempt parsing (and failing the same way) would be safer and it should also be sufficient as we are talking about how to handle a broken repository, i.e. an error case.