git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Samium Gromoff <_deepfire@feelingofgreen.ru>
Cc: git@vger.kernel.org, Daniel Barkalow <barkalow@iabervon.org>,
	Tay Ray Chuan <rctay89@gmail.com>, Mike Hommey <mh@glandium.org>
Subject: Re: Fw: git-core: SIGSEGV during {peek,ls}-remote on HTTP remotes.
Date: Sat, 31 Oct 2009 21:27:17 -0700	[thread overview]
Message-ID: <7v8weq50pm.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <20091101.010702.527849118592864646._deepfire@feelingofgreen.ru> (Samium Gromoff's message of "Sun\, 01 Nov 2009 01\:07\:02 +0300 \(MSK\)")

Samium Gromoff <_deepfire@feelingofgreen.ru> writes:

> Attached is the SEGV bugreport I sent to debian.

Thanks for a report.

There are two issues.

 * transport-helper.c::get_helper() assumes that the transport structure
   always has its remote member filled and it can get name out of it.
   This is the segv in the report.

 * Even if we work around the above issue, the helper subprocess (in this
   case, remote-curl helper) insists on being inside a git repository,  To
   satisfy ls-remote request, you do not have to be in one.

Attached is a minimum fix/work around, but this is done without being very
familiar with the current assumptions in the codepaths involved.

Issues I want area experts to consider before coming up with the final fix
are:

 - Should we fix get_helper() in transport-helper.c, instead of touching
   ls-remote.c like this patch does?

   This issue really boils down to this question: is it valid for a
   transport to have NULL in its remote field, and should all the code
   that touch transport be prepared to deal with such a transport
   structure?

 - When helping to handle ls-remote request, there is no need for the
   helper to know anything about the local state.  We probably shouldn't
   even run setup_git_directory_gently() at all in this case.  But when
   helping other kinds of request, the helper does need to know where our
   repository is.

   In general, what should the initial environment for helpers be?  Should
   they assume that they have to figure out where the git repository is
   themselves (in other words, should they assume they cannot rely on
   anything the caller does before they are called?  Would the caller
   generally have done the usual repo discovery (including chdir() to the
   toplevel), and there are some set of assumptions they can make?  If so
   what are they?


 builtin-ls-remote.c |    2 +-
 remote-curl.c       |    3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/builtin-ls-remote.c b/builtin-ls-remote.c
index 78a88f7..a8d5613 100644
--- a/builtin-ls-remote.c
+++ b/builtin-ls-remote.c
@@ -86,7 +86,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 			pattern[j - i] = p;
 		}
 	}
-	remote = nongit ? NULL : remote_get(dest);
+	remote = remote_get(dest);
 	if (remote && !remote->url_nr)
 		die("remote %s has no configured URL", dest);
 	transport = transport_get(remote, remote ? remote->url[0] : dest);
diff --git a/remote-curl.c b/remote-curl.c
index ad6a163..7c83f77 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -81,8 +81,9 @@ int main(int argc, const char **argv)
 	struct strbuf buf = STRBUF_INIT;
 	const char *url;
 	struct walker *walker = NULL;
+	int nongit = 0;
 
-	setup_git_directory();
+	setup_git_directory_gently(&nongit);
 	if (argc < 2) {
 		fprintf(stderr, "Remote needed\n");
 		return 1;

  reply	other threads:[~2009-11-01  4:27 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-10-31 22:07 Fw: git-core: SIGSEGV during {peek,ls}-remote on HTTP remotes Samium Gromoff
2009-11-01  4:27 ` Junio C Hamano [this message]
2009-11-01 14:46   ` Sverre Rabbelier
2009-11-01 20:16     ` Daniel Barkalow
2009-11-01 20:54       ` Sverre Rabbelier
2009-11-02  4:10         ` Junio C Hamano
2009-11-01 19:43   ` Daniel Barkalow
2009-11-01 20:15     ` Junio C Hamano
2009-11-01 21:19       ` Daniel Barkalow
2009-11-01 23:04         ` Jeff King
2009-11-02  0:59           ` Daniel Barkalow
2009-11-02 19:21             ` Clemens Buchacher
2009-11-01  8:19 ` Mike Hommey

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=7v8weq50pm.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=_deepfire@feelingofgreen.ru \
    --cc=barkalow@iabervon.org \
    --cc=git@vger.kernel.org \
    --cc=mh@glandium.org \
    --cc=rctay89@gmail.com \
    /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;
as well as URLs for NNTP newsgroup(s).