threads / patch / 13645

patchFix git-show-branch --current when not on a branch

Subject: [PATCH] Fix git-show-branch --current when not on a branch

## tl;dr

4 messages between May 23, 2008 and May 27, 2008. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Stephan Beyer· May 23, 2008, 23:00 UTC · lore

builtin-show-branch.c relied on resolve_ref() to return "refs/heads/foo" if on branch foo. But if not on a branch, it returns "HEAD". Hence, `head + pfxlen' (i.e. head+11) is a memory address beyond the "HEAD" string, so that further operation leads to access of uninitialized memory.

This commit fixes the bug by just not adding the "refs/heads/"-length offset. So append_one_rev() operates on "refs/heads/foo" instead of "foo", which still works. But now it also operates correctly on "HEAD".

Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
---
  builtin-show-branch.c |    3 +--
  1 files changed, 1 insertions(+), 2 deletions(-)

Show changes to builtin-show-branch.c +1 −2

diff --git a/builtin-show-branch.c b/builtin-show-branch.c
index 019abd3..412eba0 100644
--- a/builtin-show-branch.c
+++ b/builtin-show-branch.c
@@ -782,8 +782,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
  				has_head++;
  		}
  		if (!has_head) {
-			int pfxlen = strlen("refs/heads/");
-			append_one_rev(head + pfxlen);
+			append_one_rev(head);
  		}
  	}

-- 
1.5.5.1

Stephan Beyer· May 26, 2008, 12:09 UTC · re: Stephan Beyer · lore

Re: [PATCH] Fix git-show-branch --current when not on a branch

Nobody? Description too bad? Patch not working? Spam-filtered?[1]

Regards,
  Stephan

1. http://article.gmane.org/gmane.comp.version-control.git/82769

-- 
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F

Junio C Hamano· May 26, 2008, 22:05 UTC · re: Stephan Beyer · lore

Re: [PATCH] Fix git-show-branch --current when not on a branch

Stephan Beyer <s-beyer@gmx.net> writes:

Show 12 quoted lines

> builtin-show-branch.c relied on resolve_ref() to return
> "refs/heads/foo" if on branch foo.  But if not on a branch,
> it returns "HEAD".  Hence, `head + pfxlen' (i.e. head+11)
> is a memory address beyond the "HEAD" string, so that
> further operation leads to access of uninitialized memory.
>
> This commit fixes the bug by just not adding the
> "refs/heads/"-length offset.  So append_one_rev() operates
> on "refs/heads/foo" instead of "foo", which still works.
> But now it also operates correctly on "HEAD".
>
> Signed-off-by: Stephan Beyer <s-beyer@gmx.net>

Thanks for a patch and (more importantly) prodding..

Show 14 quoted lines

>  builtin-show-branch.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/builtin-show-branch.c b/builtin-show-branch.c
> index 019abd3..412eba0 100644
> --- a/builtin-show-branch.c
> +++ b/builtin-show-branch.c
> @@ -782,8 +782,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
>  				has_head++;
>  		}
>  		if (!has_head) {
> -			int pfxlen = strlen("refs/heads/");
> -			append_one_rev(head + pfxlen);
> +			append_one_rev(head);

This changes the output for normal case.  How about doing it like this instead?

 builtin-show-branch.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

Show changes to builtin-show-branch.c +2 −2

diff --git a/builtin-show-branch.c b/builtin-show-branch.c
index 019abd3..a383323 100644
--- a/builtin-show-branch.c
+++ b/builtin-show-branch.c
@@ -782,8 +782,8 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
 				has_head++;
 		}
 		if (!has_head) {
-			int pfxlen = strlen("refs/heads/");
-			append_one_rev(head + pfxlen);
+			int offset = !prefixcmp(head, "refs/heads/") ? 11 : 0;
+			append_one_rev(head + offset);
 		}
 	}

Stephan Beyer· May 27, 2008, 00:20 UTC · re: Junio C Hamano · lore

Re: [PATCH] Fix git-show-branch --current when not on a branch

Hi,

Show 16 quoted lines

> >  builtin-show-branch.c |    3 +--
> >  1 files changed, 1 insertions(+), 2 deletions(-)
> >
> > diff --git a/builtin-show-branch.c b/builtin-show-branch.c
> > index 019abd3..412eba0 100644
> > --- a/builtin-show-branch.c
> > +++ b/builtin-show-branch.c
> > @@ -782,8 +782,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
> >  				has_head++;
> >  		}
> >  		if (!has_head) {
> > -			int pfxlen = strlen("refs/heads/");
> > -			append_one_rev(head + pfxlen);
> > +			append_one_rev(head);
> 
> This changes the output for normal case.

It does?  How?

I cite my commit message:

> [...] So append_one_rev() operates
> on "refs/heads/foo" instead of "foo", which still works.
> But now it also operates correctly on "HEAD".

And I still think it's true.
I diff'ed the outputs of
	git-show-branch --current
AND (just to go sure)
	git-show-branch
on git.next
before and after the patch:
	No difference.

> +			int offset = !prefixcmp(head, "refs/heads/") ? 11 : 0;

Your version is the intuitive change and, of course, right, but I don't see why we should do the prefixcmp() when it does not change the resulting behavior in any way.

Regards,
  Stephan

-- 
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F