Re: [PATCH 2/2] fsck: handle bad trees like other errors

[Junio C Hamano <gitster@pobox.com>]

David Turner <novalis@novalis.org> writes:

> @@ -535,7 +536,10 @@ static int fsck_tree(struct tree *item, struct fsck_options *options)
>  	unsigned o_mode;
>  	const char *o_name;
>  
> -	init_tree_desc(&desc, item->buffer, item->size);
> +	if (init_tree_desc_gently(&desc, item->buffer, item->size)) {
> +		retval += report(options, &item->object, FSCK_MSG_BAD_TREE, "cannot be parsed as a tree");
> +		return retval;
> +	}

Good.  If BAD_TREE is being ignored, this may report a non-error,
but we won't descend into the unreadable tree so it is OK.

> @@ -556,7 +560,10 @@ static int fsck_tree(struct tree *item, struct fsck_options *options)
>  			       is_hfs_dotgit(name) ||
>  			       is_ntfs_dotgit(name));
>  		has_zero_pad |= *(char *)desc.buffer == '0';
> -		update_tree_entry(&desc);
> +		if (update_tree_entry_gently(&desc)) {
> +			retval += report(options, &item->object, FSCK_MSG_BAD_TREE, "cannot be parsed as a tree");
> +			break;
> +		}

Likewise; breaking out of the loop will stop us from reading further
into the corrupted tree data, so this is good.

> @@ -597,7 +604,6 @@ static int fsck_tree(struct tree *item, struct fsck_options *options)
>  		o_name = name;
>  	}
>  
> -	retval = 0;

Good code hygiene that you moved this to the very top where it is
defined, so anybody before this step can set it if it wants to.

Reading purely from the text of this function, it was surprising
that you can do without a gently variant of tree_entry_extract(),
but it merely reads into two variables and does not do any error
detection (which happens all in the caller), so it is not at all
surprising after all ;-)

I didn't see anything objectionable in this patch.  Thanks for
working on this.