From: Jonathan Nieder <jrnieder@gmail.com>
Date: Wed, 19 Jul 2017 23:25:41 GMT
Subject: Re: [PATCH] fsck: remove redundant parse_tree() invocation
Message-ID: <20170719232541.GA13924@aiede.mtv.corp.google.com>
In-Reply-To: <20170718222848.1453-1-jonathantanmy@google.com>

Hi,

Jonathan Tan wrote:

> If obj->type == OBJ_TREE, an invocation of fsck_walk() will invoke
> parse_tree() and return quickly if that returns nonzero, so it is of no
> use for traverse_one_object() to invoke parse_tree() in this situation
> before invoking fsck_walk(). Remove that code.

I like the diffstat.  I'm trying to figure out what the old code was
trying to do, since that will make it easier to understand this
change.

fsck_walk_tree calls parse_tree, so why did we need to parse it in
advance?  Was it just about handling the error differently when it
fails to parse?

It appears this code comes from
https://public-inbox.org/git/20080214090013.GK24004@spearce.org/:

 +	if (parse_tree(item) < 0)
 +		return; /* error already displayed */
 +
 +	init_tree_desc(&desc, item->buffer, item->size);

At that point the parse_tree call was not redundant.  Later it must
have been amended and cleaned up to use fsck_walk.

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Thanks for cleaning up.