Re: [PATCH 3/3] commit-graph.c: handle corrupt/missing trees
- From
Jeff King <peff@peff.net>
- Date
- Sep 6, 2019, 17:30 UTC
- Message-ID
- <20190906173043.GD23181@sigill.intra.peff.net>
- In-Reply-To
- <xmqq5zm5nydu.fsf@gitster-ct.c.googlers.com>
On Fri, Sep 06, 2019 at 10:11:09AM -0700, Junio C Hamano wrote:
Show 20 quoted lines
> > Is there even a single caller that is prepared to react to NULL? > > > > Answer. There is a single hit inside fsck.c that wants to report > > an error without killing ourselves in fsck_commit_buffer(). I > > however doubt its use of get_commit_tree() is correct in the > > first place. The function is about validating the commit object > > payload manually, without trusting the result of parse_commit(), > > and it does read the object name of the tree object; the call to > > get_commit_tree() used for reporting the error there should > > probably become has_object() on the tree_oid. > > At least we need to ensure, not just has_object(), but the object > indeed claims to be a tree object. It is OK if it is a corrupt > tree object---we'll catch its brokenness separately anyway. > > Hmm, the should we also tolerate the pointed object to be broken > in a way that it is not even able to claim to be a tree object? > That would mean that has_object() is sufficient to check here. > > OK, so...
We'd do that check later, during fsck_walk_commit(). Which ironically calls get_commit_tree() without checking for NULL, and would likely segfault. ;)
-Peff