Re: Weird problem with long $PATH and alternates (bisected)
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 26, 2008, 18:07 UTC
- Message-ID
- <7v63nf5iy1.fsf@gitster.siamese.dyndns.org>
- In-Reply-To
- <4904AE8D.9090706@lsrfire.ath.cx>
René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
Show 32 quoted lines
> Junio C Hamano schrieb: >> "Mikael Magnusson" <mikachu@gmail.com> writes: >> >>> % mkdir 1; cd 1 >>> % echo > a; git add a; git commit -m a >>> % cd .. >>> % git clone -s 1 2 >>> % git push . master:master >>> fatal: Could not switch to >>> '/tmp/a/1/.git/objects/n:/usr/games/bin:/usr/local/ipod-chain' >>> fatal: The remote end hung up unexpectedly >> >> I think I see a bug in foreach_alt_odb() to add_refs_from_alternate() >> callchain, but I cannot explain why the contents of $PATH leaks to the >> error message. > > With the following patch, I can no longer reproduce the problem. Does it > work fo you, too? > > Thanks, > René > > diff --git a/sha1_file.c b/sha1_file.c > index ab2b520..8044e9c 100644 > --- a/sha1_file.c > +++ b/sha1_file.c > @@ -269,7 +269,7 @@ static int link_alt_odb_entry(const char * entry, int len, const char * relative > entlen += base_len; > pfxlen += base_len; > } > - ent = xmalloc(sizeof(*ent) + entlen); > + ent = xcalloc(1, sizeof(*ent) + entlen);
Ah, that would explain the "filled with garbage from $PATH" issue, but I don't think it fixes the fundamental issue.
In the alternate_object_database structure, ent->base[] is a buffer the users can use to form pathnames to loose objects, and ent->name is a pointer into that buffer (it points at one beyond ".git/objects/"). If you get a call to add_refs_from_alternate() after somebody used the entry (has_loose_object() has been called, for example), *ent->name would not be NUL, and ent->base[] won't be the path to the object store.
This caller is expecting to read the path to the object store in ent->base[]; it needs to NUL terminate the buffer if it wants to.
I think the previous patch to sha1_file.c, while it may fix the issue, is not quite nice. Here is a replacement that should work.
builtin-receive-pack.c | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-)
diff --git c/builtin-receive-pack.c w/builtin-receive-pack.c index 45e3cd9..9f60f31 100644 --- c/builtin-receive-pack.c +++ w/builtin-receive-pack.c @@ -466,12 +466,17 @@ static int delete_only(struct command *cmd) static int add_refs_from_alternate(struct alternate_object_database *e, void *unused) { - char *other = xstrdup(make_absolute_path(e->base)); - size_t len = strlen(other); + char *other; + size_t len; struct remote *remote; struct transport *transport; const struct ref *extra; + e->name[-1] = '\0'; + other = xstrdup(make_absolute_path(e->base)); + e->name[-1] = '/'; + len = strlen(other); + while (other[len-1] == '/') other[--len] = '\0'; if (len < 8 || memcmp(other + len - 8, "/objects", 8))