public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed

From: Junio C Hamano <gitster@pobox•com>
To: Jeff King <peff@peff•net>
Cc: git@vger•kernel.org
Subject: Re: [PATCH] open_sha1_file: report "most interesting" errno
Date: Thu, 15 May 2014 10:02:06 -0700	[thread overview]
Message-ID: <xmqqbnuzgelt.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <20140515085405.GA27033@sigill.intra.peff.net> (Jeff King's message of "Thu, 15 May 2014 04:54:06 -0400")

Jeff King <peff@peff•net> writes:

> When we try to open a loose object file, we first attempt to
> open in the local object database, and then try any
> alternates. This means that the errno value when we return
> will be from the last place we looked (and due to the way
> the code is structured, simply ENOENT if we do not have have
> any alternates).
>
> This can cause confusing error messages, as read_sha1_file
> checks for ENOENT when reporting a missing object. If errno
> is something else, we report that. If it is ENOENT, but
> has_loose_object reports that we have it, then we claim the
> object is corrupted. For example:
>
>     $ chmod 0 .git/objects/??/*
>     $ git rev-list --all
>     fatal: loose object b2d6fab18b92d49eac46dc3c5a0bcafabda20131 (stored in .git/objects/b2/d6fab18b92d49eac46dc3c5a0bcafabda20131) is corrupt

Hmmmmmmmm.  So we keep track of a more interesting errno we get from
some other place than what we get for this local loose object, and
we no longer give this message pointing at the local loose
object---is that the idea?

What I am wondering is that this report we give in the new code

>     $ git rev-list --all
>     fatal: failed to read object b2d6fab18b92d49eac46dc3c5a0bcafabda20131: Permission denied

may want to say which of the various possible places we saw this
most interesting errno, which I think was the original motivation
came from e8b15e61 (sha1_file: Show the the type and path to corrupt
objects, 2010-06-10) that added "(stored in ...)".

But that may involve a larger surgery, and I definitely do not want
to add unnecessary logic in the common-case codepath to keep track
of pieces of information that are only used in the error codepath,
so it smells like that this is the best fix to the issue the commit
message describes.

Thanks.

>
> Signed-off-by: Jeff King <peff@peff•net>
> ---
>  sha1_file.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/sha1_file.c b/sha1_file.c
> index 3e9f55f..34d527f 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -1437,19 +1437,23 @@ static int open_sha1_file(const unsigned char *sha1)
>  {
>  	int fd;
>  	struct alternate_object_database *alt;
> +	int most_interesting_errno;
>  
>  	fd = git_open_noatime(sha1_file_name(sha1));
>  	if (fd >= 0)
>  		return fd;
> +	most_interesting_errno = errno;
>  
>  	prepare_alt_odb();
> -	errno = ENOENT;
>  	for (alt = alt_odb_list; alt; alt = alt->next) {
>  		fill_sha1_path(alt->name, sha1);
>  		fd = git_open_noatime(alt->base);
>  		if (fd >= 0)
>  			return fd;
> +		if (most_interesting_errno == ENOENT)
> +			most_interesting_errno = errno;
>  	}
> +	errno = most_interesting_errno;
>  	return -1;
>  }

next prev parent reply	other threads:[~2014-05-15 17:02 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-15  8:54 [PATCH] open_sha1_file: report "most interesting" errno Jeff King
2014-05-15 17:02 ` Junio C Hamano [this message]
2014-05-15 19:11   ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqbnuzgelt.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=peff@peff$(echo .)net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox