Re: [PATCH] Correctly report corrupted objects

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 20, 2011, 21:05 UTC
Message-ID: <7vsjwnqoao.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20110120201220.GD12418@atjola.homenet>

Björn Steinbrink <B.Steinbrink@gmx.de> writes:

Show 12 quoted lines

> The errno check added in commit 3ba7a06 "A loose object is not corrupt
> if it cannot be read due to EMFILE" only checked for whether errno is
> not ENOENT and thus incorrectly treated "no error" as an error
> condition.
>
> Because of that, it never reached the code path that would report that
> the object is corrupted and instead caused funny errors like:
>
>   fatal: failed to read object 333c4768ce595793fdab1ef3a036413e2a883853: Success
>
> So we have to extend the check to cover the case in which the object
> file was successfully read, but its contents are corrupted.

Hmm, what is the exact code path that read_object() callchain fails to set errno when it returns a NULL? It is unclear from the above description. Is there a funny object replacement involved?

Show 21 quoted lines

> Reported-by: Will Palmer <wmpalmer@gmail.com>
> Signed-off-by: Björn Steinbrink <B.Steinbrink@gmx.de>
> ---
>  sha1_file.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/sha1_file.c b/sha1_file.c
> index 1cafdfa..d86a8db 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -2141,7 +2141,7 @@ void *read_sha1_file_repl(const unsigned char *sha1,
>  		return data;
>  	}
>  
> -	if (errno != ENOENT)
> +	if (errno && errno != ENOENT)
>  		die_errno("failed to read object %s", sha1_to_hex(sha1));
>  
>  	/* die if we replaced an object with one that does not exist */
> -- 
> 1.7.4.rc2.18.gb20e9