threads / patch / 18964

patchBUG: git push on an empty clone segfaults.

Subject: [PATCH] BUG: git push on an empty clone segfaults.

## tl;dr

11 messages between Apr 20, 2009 and Apr 20, 2009. Diffs are folded; open one to read it.

replies: 10people: 5as markdown or json

Matthieu Moy· Apr 20, 2009, 09:48 UTC · lore

Ideally, "git push" from an empty repository to another empty one should be a no-op, or perhaps should error out cleanly. Currently, it just segfaults.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 t/t5701-clone-local.sh |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

Show changes to t/t5701-clone-local.sh +10 −0

diff --git a/t/t5701-clone-local.sh b/t/t5701-clone-local.sh
index 3559d17..7c6ef4c 100755
--- a/t/t5701-clone-local.sh
+++ b/t/t5701-clone-local.sh
@@ -132,4 +132,14 @@ test_expect_success 'clone empty repository' '
 	 test $actual = $expected)
 '
 
+test_expect_failure 'clone empty repository, and then push should not segfault.' '
+	cd "$D" &&
+	rm -fr empty/ empty-clone/ &&
+	mkdir empty &&
+	(cd empty && git init) &&
+	git clone empty empty-clone &&
+	cd empty-clone &&
+	test_must_fail git push
+'
+
 test_done

-- 
1.6.2.2.449.g92961.dirty

Johannes Schindelin· Apr 20, 2009, 10:20 UTC · re: Matthieu Moy · lore

[PATCH] Fix uninitialized memory in get_local_heads()

In 454e202(move duplicated get_local_heads() to remote.c), a static local variable was moved into a function, but not initialized.

This resulted in a crash when trying to push from an empty repository.

Noticed by Matthieu Moy.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	On Mon, 20 Apr 2009, Matthieu Moy wrote:

	> Ideally, "git push" from an empty repository to another empty 
	> one should be a no-op, or perhaps should error out cleanly.
	> Currently, it just segfaults.

	Good catch.  This patch goes on top of yours.

	/me is running valgrind now.

 remote.c               |    2 +-
 t/t5701-clone-local.sh |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Show changes to 2 files +2 −2

remote.c, t/t5701-clone-local.sh

diff --git a/remote.c b/remote.c
index a06761a..e4c89b8 100644
--- a/remote.c
+++ b/remote.c
@@ -1504,7 +1504,7 @@ static int one_local_ref(const char *refname, const unsigned char *sha1, int fla
 
 struct ref *get_local_heads(void)
 {
-	struct ref *local_refs, **local_tail = &local_refs;
+	struct ref *local_refs = NULL, **local_tail = &local_refs;
 	for_each_ref(one_local_ref, &local_tail);
 	return local_refs;
 }
diff --git a/t/t5701-clone-local.sh b/t/t5701-clone-local.sh
index 7c6ef4c..f26b511 100755
--- a/t/t5701-clone-local.sh
+++ b/t/t5701-clone-local.sh
@@ -132,7 +132,7 @@ test_expect_success 'clone empty repository' '
 	 test $actual = $expected)
 '
 
-test_expect_failure 'clone empty repository, and then push should not segfault.' '
+test_expect_success 'clone empty repository, and then push should not segfault.' '
 	cd "$D" &&
 	rm -fr empty/ empty-clone/ &&
 	mkdir empty &&

-- 
1.6.2.1.493.g67cf3

Jay Soffian· Apr 20, 2009, 13:51 UTC · re: Johannes Schindelin · lore

Re: [PATCH] Fix uninitialized memory in get_local_heads()

On Mon, Apr 20, 2009 at 6:20 AM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 9 quoted lines

>
> In 454e202(move duplicated get_local_heads() to remote.c), a static
> local variable was moved into a function, but not initialized.
>
> This resulted in a crash when trying to push from an empty repository.
>
> Noticed by Matthieu Moy.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Doh. Thank you for the fix.

j.

Junio C Hamano· Apr 20, 2009, 10:33 UTC · re: Matthieu Moy · lore

Re: [PATCH] BUG: git push on an empty clone segfaults.

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> Ideally, "git push" from an empty repository to another empty one
> should be a no-op, or perhaps should error out cleanly. Currently, it
> just segfaults.

Didn't we see this fixed by Nguyễn with 55f0566 (get_local_heads(): do not return random pointer if there is no head, 2009-04-17)?

Johannes Schindelin· Apr 20, 2009, 10:55 UTC · re: Junio C Hamano · lore

Re: [PATCH] BUG: git push on an empty clone segfaults.

Hi,

On Mon, 20 Apr 2009, Junio C Hamano wrote:

Show 8 quoted lines

> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
> 
> > Ideally, "git push" from an empty repository to another empty one
> > should be a no-op, or perhaps should error out cleanly. Currently, it
> > just segfaults.
> 
> Didn't we see this fixed by Nguyễn with 55f0566 (get_local_heads(): do not
> return random pointer if there is no head, 2009-04-17)?

I fetched from repo.or.cz and tested with 'master', and it was broken. Apparently git://repo.or.cz/git.git is lagging behind by 5 days. Pasky?

Ciao, Dscho

Junio C Hamano· Apr 20, 2009, 11:13 UTC · re: Johannes Schindelin · lore

Re: [PATCH] BUG: git push on an empty clone segfaults.

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 13 quoted lines

> On Mon, 20 Apr 2009, Junio C Hamano wrote:
>
>> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>> 
>> > Ideally, "git push" from an empty repository to another empty one
>> > should be a no-op, or perhaps should error out cleanly. Currently, it
>> > just segfaults.
>> 
>> Didn't we see this fixed by Nguyễn with 55f0566 (get_local_heads(): do not
>> return random pointer if there is no head, 2009-04-17)?
>
> I fetched from repo.or.cz and tested with 'master', and it was broken.  
> Apparently git://repo.or.cz/git.git is lagging behind by 5 days.  Pasky?

Unfortunately, that repository is outside of my direct control.

I push into the following four repositories:

	k.org's master repository
	git://repo.or.cz/alt-git.git/
        git://git.sourceforge.jp/gitroot/git-core/git.git
	git://git-core.git.sourceforge.net/gitroot/git-core

The k.org's master repository is mirrored (with small lag) to:

	git://git.kernel.org/pub/scm/git/git.git

I suspect that Pasky mirrors that mirror (with some lag) to:

        git://repo.or.cz/git.git

There may be some other mirrors of mirrors I do not know about.

I'd guess that people in the US are better off going to git.kernel.org, Europeans to git://repo.or.cz/alt-git.git, and East Asians to git.sourceforge.jp/.

I've already written about this elsewhere [*1*], but the sourceforge ones are partial; they only have maint and master (and tags).

[Footnote]

*1* http://gitster.livejournal.com/31668.html

Petr Baudis· Apr 20, 2009, 13:55 UTC · re: Johannes Schindelin · lore

Re: [PATCH] BUG: git push on an empty clone segfaults.

Hi,

On Mon, Apr 20, 2009 at 12:55:10PM +0200, Johannes Schindelin wrote:

> I fetched from repo.or.cz and tested with 'master', and it was broken.  
> Apparently git://repo.or.cz/git.git is lagging behind by 5 days.  Pasky?

  yes, I noticed that the mirroring got hanging on some dead repository
earlier today and restarted it, but it might take a while until a full
mirror cycle refreshes everything.

  Sorry about this.

				Petr "Pasky" Baudis

Johannes Schindelin· Apr 20, 2009, 14:30 UTC · re: Petr Baudis · lore

Re: [PATCH] BUG: git push on an empty clone segfaults.

Hi,

On Mon, 20 Apr 2009, Petr Baudis wrote:

Show 8 quoted lines

> On Mon, Apr 20, 2009 at 12:55:10PM +0200, Johannes Schindelin wrote:
> > I fetched from repo.or.cz and tested with 'master', and it was broken.  
> > Apparently git://repo.or.cz/git.git is lagging behind by 5 days.  
> > Pasky?
> 
>   yes, I noticed that the mirroring got hanging on some dead repository 
> earlier today and restarted it, but it might take a while until a full 
> mirror cycle refreshes everything.

I always wondered why it should make sense to have a mirror git.git, and in addition a repository alt-git.git that Junio pushes to.

Maybe it is time to fix that?

I mean, I am hardly a Git newbie, and even I got burnt.

Ciao, Dscho

Petr Baudis· Apr 20, 2009, 20:46 UTC · re: Johannes Schindelin · lore

Re: [PATCH] BUG: git push on an empty clone segfaults.

On Mon, Apr 20, 2009 at 04:30:38PM +0200, Johannes Schindelin wrote:

Show 17 quoted lines

> On Mon, 20 Apr 2009, Petr Baudis wrote:
> 
> > On Mon, Apr 20, 2009 at 12:55:10PM +0200, Johannes Schindelin wrote:
> > > I fetched from repo.or.cz and tested with 'master', and it was broken.  
> > > Apparently git://repo.or.cz/git.git is lagging behind by 5 days.  
> > > Pasky?
> > 
> >   yes, I noticed that the mirroring got hanging on some dead repository 
> > earlier today and restarted it, but it might take a while until a full 
> > mirror cycle refreshes everything.
> 
> I always wondered why it should make sense to have a mirror git.git, and 
> in addition a repository alt-git.git that Junio pushes to.
> 
> Maybe it is time to fix that?
> 
> I mean, I am hardly a Git newbie, and even I got burnt.

Sure, I can fix that, making alt-git.git a soft-link to git.git (or, specifically, making refs/ and objects/ symlinks). Junio even might not have to change his push scripts. ;-)

-- 
				Petr "Pasky" Baudis
The average, healthy, well-adjusted adult gets up at seven-thirty
in the morning feeling just terrible. -- Jean Kerr

Matthieu Moy· Apr 20, 2009, 11:07 UTC · re: Junio C Hamano · lore

Re: [PATCH] BUG: git push on an empty clone segfaults.

Junio C Hamano <gitster@pobox.com> writes:

Show 8 quoted lines

> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>
>> Ideally, "git push" from an empty repository to another empty one
>> should be a no-op, or perhaps should error out cleanly. Currently, it
>> just segfaults.
>
> Didn't we see this fixed by Nguyễn with 55f0566 (get_local_heads(): do not
> return random pointer if there is no head, 2009-04-17)?

Oops, right, I wasn't up to date. Still, the test-case is relevant, I'll resend with test_expect_success.

-- 
Matthieu

Matthieu Moy· Apr 20, 2009, 11:09 UTC · re: Junio C Hamano · lore

[PATCH] clone: add test for push on an empty clone.

Commit 55f0566 (get_local_heads(): do not return random pointer if there is no head, 2009-04-17) fixed a segfault for git push, this patch adds a test-case to avoid future regressions.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 t/t5701-clone-local.sh |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

Show changes to t/t5701-clone-local.sh +10 −0

diff --git a/t/t5701-clone-local.sh b/t/t5701-clone-local.sh
index 3559d17..f26b511 100755
--- a/t/t5701-clone-local.sh
+++ b/t/t5701-clone-local.sh
@@ -132,4 +132,14 @@ test_expect_success 'clone empty repository' '
 	 test $actual = $expected)
 '
 
+test_expect_success 'clone empty repository, and then push should not segfault.' '
+	cd "$D" &&
+	rm -fr empty/ empty-clone/ &&
+	mkdir empty &&
+	(cd empty && git init) &&
+	git clone empty empty-clone &&
+	cd empty-clone &&
+	test_must_fail git push
+'
+
 test_done

-- 
1.6.2.2.449.g92961.dirty