Git development
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Marinescu Paul dan <pauldan.marinescu@epfl.ch>
Cc: Junio C Hamano <gitster@pobox.com>,
	"git@vger.kernel.org" <git@vger.kernel.org>
Subject: Re: [BUG] crash on make test
Date: Fri, 4 Dec 2009 05:35:57 -0500	[thread overview]
Message-ID: <20091204103557.GC27495@coredump.intra.peff.net> (raw)
In-Reply-To: <D6F784B72498304C93A8A4691967698E8EE2C44FE2@REX2.intranet.epfl.ch>

On Thu, Dec 03, 2009 at 04:38:58PM +0100, Marinescu Paul dan wrote:

> I got a crash on git's make test with and a core file in ./t/trash
> directory.t4200-rerere/ The problem seems to be in garbage_collect
> (builtin-rerere.c) where readdir can be called with a null pointer

Hmm. The problematic code is pretty straightforward to see, but what I
don't understand is how you managed to trigger it. The code path to get
there makes sure that the rr-cache directory exists (because we check
whether rerere is enabled, which means either rr-cache exists, or
rerere.enabled is set, and in the latter case we mkdir the directory).

I can see how it might happen if rr-cache exists but isn't readable. I
can easily trigger a segfault with:

  mkdir .git/rr-cache
  sudo chown root .git/rr-cache
  sudo chmod 0700 .git/rr-cache
  git gc

and we should definitely fix that (patch below).

But how do you end up with this situation in the middle of the test
suite? And if it is not a permissions problem, but that the directory is
missing, that would indicate something randomly deleting files while
you're running the test.

I think we should apply the patch below to maint, as this is something
that can come up due to permissions problems. But I fear it won't
actually fix the test failure you are seeing; you will just see it die()
instead of segfaulting. However, the value of errno should give us a
clue about what is happening, so please try running the test again with
this patch.

-- >8 --
Subject: [PATCH] rerere: don't segfault on failure to open rr-cache

The rr-cache directory should always exist if we are doing
garbage collection (earlier code paths check this
explicitly), but we may not necessarily succeed in opening
it (for example, due to permissions problems). In that case,
we should print an error message rather than simply
segfaulting.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin-rerere.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/builtin-rerere.c b/builtin-rerere.c
index 343d6cd..2be9ffb 100644
--- a/builtin-rerere.c
+++ b/builtin-rerere.c
@@ -48,6 +48,8 @@ static void garbage_collect(struct string_list *rr)
 
 	git_config(git_rerere_gc_config, NULL);
 	dir = opendir(git_path("rr-cache"));
+	if (!dir)
+		die_errno("unable to open rr-cache directory");
 	while ((e = readdir(dir))) {
 		if (is_dot_or_dotdot(e->d_name))
 			continue;
-- 
1.6.6.rc1.18.ga777f.dirty

  reply	other threads:[~2009-12-04 10:36 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <D6F784B72498304C93A8A4691967698E8EE2C44FE1@REX2.intranet.epfl.ch>
2009-12-03 15:38 ` [BUG] crash on make test Marinescu Paul dan
2009-12-04 10:35   ` Jeff King [this message]
2009-12-04 17:12     ` Junio C Hamano

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=20091204103557.GC27495@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pauldan.marinescu@epfl.ch \
    /path/to/YOUR_REPLY

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

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox