{"thread":{"id":"13645","subject":"[PATCH] Fix git-show-branch --current when not on a branch","startedAt":"2008-05-23T23:00:38Z","lastAt":"2008-05-27T00:20:51Z","messageCount":4,"participants":["Stephan Beyer","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"77611","messageId":"alpine.DEB.1.10.0805240049400.3980@euler.pkqs.net","threadId":"13645","inReplyTo":null,"subject":"[PATCH] Fix git-show-branch --current when not on a branch","fromName":"Stephan Beyer","fromEmail":"s-beyer@gmx.net","sentAt":"2008-05-23T23:00:38Z","receivedAt":"2008-05-23T23:00:38Z","isPatch":true,"sender":{"key":"s-beyer@gmx.net","avatar":"https://avatars.githubusercontent.com/u/143889?v=4"},"body":"builtin-show-branch.c relied on resolve_ref() to return\n\"refs/heads/foo\" if on branch foo. But if not on a branch,\nit returns \"HEAD\". Hence, `head + pfxlen' (i.e. head+11)\nis a memory address beyond the \"HEAD\" string, so that\nfurther operation leads to access of uninitialized memory.\n\nThis commit fixes the bug by just not adding the\n\"refs/heads/\"-length offset. So append_one_rev() operates\non \"refs/heads/foo\" instead of \"foo\", which still works.\nBut now it also operates correctly on \"HEAD\".\n\nSigned-off-by: Stephan Beyer \n---\n builtin-show-branch.c | 3 +--\n 1 files changed, 1 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-show-branch.c b/builtin-show-branch.c\nindex 019abd3..412eba0 100644\n--- a/builtin-show-branch.c\n+++ b/builtin-show-branch.c\n@@ -782,8 +782,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \t\t\t\thas_head++;\n \t\t}\n \t\tif (!has_head) {\n-\t\t\tint pfxlen = strlen(\"refs/heads/\");\n-\t\t\tappend_one_rev(head + pfxlen);\n+\t\t\tappend_one_rev(head);\n \t\t}\n \t}\n\n-- \n1.5.5.1\n"},{"id":"77768","messageId":"20080526120924.GB5474@leksak.fem-net","threadId":"13645","inReplyTo":"alpine.DEB.1.10.0805240049400.3980@euler.pkqs.net","subject":"Re: [PATCH] Fix git-show-branch --current when not on a branch","fromName":"Stephan Beyer","fromEmail":"s-beyer@gmx.net","sentAt":"2008-05-26T12:09:24Z","receivedAt":"2008-05-26T12:09:24Z","isPatch":true,"sender":{"key":"s-beyer@gmx.net","avatar":"https://avatars.githubusercontent.com/u/143889?v=4"},"body":"Nobody?\nDescription too bad? Patch not working? Spam-filtered?[1]\n\nRegards,\n Stephan\n\n1. http://article.gmane.org/gmane.comp.version-control.git/82769\n\n-- \nStephan Beyer , PGP 0x6EDDD207FCC5040F\n"},{"id":"77808","messageId":"7vhcck4t6z.fsf@gitster.siamese.dyndns.org","threadId":"13645","inReplyTo":"alpine.DEB.1.10.0805240049400.3980@euler.pkqs.net","subject":"Re: [PATCH] Fix git-show-branch --current when not on a branch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-26T22:05:40Z","receivedAt":"2008-05-26T22:05:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stephan Beyer writes:\n\n> builtin-show-branch.c relied on resolve_ref() to return\n> \"refs/heads/foo\" if on branch foo. But if not on a branch,\n> it returns \"HEAD\". Hence, `head + pfxlen' (i.e. head+11)\n> is a memory address beyond the \"HEAD\" string, so that\n> further operation leads to access of uninitialized memory.\n>\n> This commit fixes the bug by just not adding the\n> \"refs/heads/\"-length offset. So append_one_rev() operates\n> on \"refs/heads/foo\" instead of \"foo\", which still works.\n> But now it also operates correctly on \"HEAD\".\n>\n> Signed-off-by: Stephan Beyer \n\nThanks for a patch and (more importantly) prodding..\n\n> builtin-show-branch.c | 3 +--\n> 1 files changed, 1 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin-show-branch.c b/builtin-show-branch.c\n> index 019abd3..412eba0 100644\n> --- a/builtin-show-branch.c\n> +++ b/builtin-show-branch.c\n> @@ -782,8 +782,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n> \t\t\t\thas_head++;\n> \t\t}\n> \t\tif (!has_head) {\n> -\t\t\tint pfxlen = strlen(\"refs/heads/\");\n> -\t\t\tappend_one_rev(head + pfxlen);\n> +\t\t\tappend_one_rev(head);\n\nThis changes the output for normal case. How about doing it like this instead?\n\n builtin-show-branch.c | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-show-branch.c b/builtin-show-branch.c\nindex 019abd3..a383323 100644\n--- a/builtin-show-branch.c\n+++ b/builtin-show-branch.c\n@@ -782,8 +782,8 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \t\t\t\thas_head++;\n \t\t}\n \t\tif (!has_head) {\n-\t\t\tint pfxlen = strlen(\"refs/heads/\");\n-\t\t\tappend_one_rev(head + pfxlen);\n+\t\t\tint offset = !prefixcmp(head, \"refs/heads/\") ? 11 : 0;\n+\t\t\tappend_one_rev(head + offset);\n \t\t}\n \t}\n \n"},{"id":"77823","messageId":"20080527002051.GA13683@leksak.fem-net","threadId":"13645","inReplyTo":"7vhcck4t6z.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Fix git-show-branch --current when not on a branch","fromName":"Stephan Beyer","fromEmail":"s-beyer@gmx.net","sentAt":"2008-05-27T00:20:51Z","receivedAt":"2008-05-27T00:20:51Z","isPatch":true,"sender":{"key":"s-beyer@gmx.net","avatar":"https://avatars.githubusercontent.com/u/143889?v=4"},"body":"Hi,\n\n> > builtin-show-branch.c | 3 +--\n> > 1 files changed, 1 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/builtin-show-branch.c b/builtin-show-branch.c\n> > index 019abd3..412eba0 100644\n> > --- a/builtin-show-branch.c\n> > +++ b/builtin-show-branch.c\n> > @@ -782,8 +782,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n> > \t\t\t\thas_head++;\n> > \t\t}\n> > \t\tif (!has_head) {\n> > -\t\t\tint pfxlen = strlen(\"refs/heads/\");\n> > -\t\t\tappend_one_rev(head + pfxlen);\n> > +\t\t\tappend_one_rev(head);\n> \n> This changes the output for normal case.\n\nIt does? How?\n\nI cite my commit message:\n> [...] So append_one_rev() operates\n> on \"refs/heads/foo\" instead of \"foo\", which still works.\n> But now it also operates correctly on \"HEAD\".\n\nAnd I still think it's true.\nI diff'ed the outputs of\n\tgit-show-branch --current\nAND (just to go sure)\n\tgit-show-branch\non git.next\nbefore and after the patch:\n\tNo difference.\n\n> +\t\t\tint offset = !prefixcmp(head, \"refs/heads/\") ? 11 : 0;\n\nYour version is the intuitive change and, of course, right,\nbut I don't see why we should do the prefixcmp() when it does\nnot change the resulting behavior in any way.\n\nRegards,\n Stephan\n\n-- \nStephan Beyer , PGP 0x6EDDD207FCC5040F\n"}]}