{"thread":{"id":"45234","subject":"[PATCH] interpret_branch_name(): handle auto-namelen for @{-1}","startedAt":"2017-02-27T09:25:47Z","lastAt":"2017-02-27T19:54:34Z","messageCount":3,"participants":["Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"312708","messageId":"20170227092540.ob2oy4qpgfukqwtc@sigill.intra.peff.net","threadId":"45234","inReplyTo":null,"subject":"[PATCH] interpret_branch_name(): handle auto-namelen for @{-1}","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-02-27T09:25:40Z","receivedAt":"2017-02-27T09:25:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The interpret_branch_name() function takes a ptr/len pair\nfor the name, but you can pass \"0\" for \"namelen\", which will\ncause it to check the length with strlen().\n\nHowever, before we do that auto-namelen magic, we call\ninterpret_nth_prior_checkout(), which gets fed the bogus\n\"0\". This was broken by 8cd4249c4 (interpret_branch_name:\nalways respect \"namelen\" parameter, 2014-01-15).  Though to\nbe fair to that commit, it was broken in the _opposite_\ndirection before, where we would always treat \"name\" as a\nstring even if a length was passed.\n\nYou can see the bug with \"git log -g @{-1}\". That code path\nalways passes \"0\", and without this patch it cannot figure\nout which branch's reflog to show.\n\nWe can fix it by a small reordering of the code.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nNoticed while digging on the nearby thread:\n\n  http://public-inbox.org/git/20170227090233.uk7dfruggytgmuw2@sigill.intra.peff.net/\n\n sha1_name.c         | 3 ++-\n t/t0100-previous.sh | 8 ++++++++\n 2 files changed, 10 insertions(+), 1 deletion(-)\n\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 73a915ff1..9b5d14b4b 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -1263,11 +1263,12 @@ int interpret_branch_name(const char *name, int namelen, struct strbuf *buf)\n {\n \tchar *at;\n \tconst char *start;\n-\tint len = interpret_nth_prior_checkout(name, namelen, buf);\n+\tint len;\n \n \tif (!namelen)\n \t\tnamelen = strlen(name);\n \n+\tlen = interpret_nth_prior_checkout(name, namelen, buf);\n \tif (!len) {\n \t\treturn len; /* syntax Ok, not enough switches */\n \t} else if (len > 0) {\ndiff --git a/t/t0100-previous.sh b/t/t0100-previous.sh\nindex e0a694023..58c0b7e9b 100755\n--- a/t/t0100-previous.sh\n+++ b/t/t0100-previous.sh\n@@ -56,5 +56,13 @@ test_expect_success 'merge @{-100} before checking out that many branches yet' '\n \ttest_must_fail git merge @{-100}\n '\n \n+test_expect_success 'log -g @{-1}' '\n+\tgit checkout -b last_branch &&\n+\tgit checkout -b new_branch &&\n+\techo \"last_branch@{0}\" >expect &&\n+\tgit log -g --format=%gd @{-1} >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n \n-- \n2.12.0.624.gbb1b07a2c\n"},{"id":"312711","messageId":"20170227093122.3jdb2b62hlbbio5r@sigill.intra.peff.net","threadId":"45234","inReplyTo":"20170227092540.ob2oy4qpgfukqwtc@sigill.intra.peff.net","subject":"Re: [PATCH] interpret_branch_name(): handle auto-namelen for @{-1}","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-02-27T09:31:22Z","receivedAt":"2017-02-27T09:58:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 27, 2017 at 04:25:40AM -0500, Jeff King wrote:\n\n> However, before we do that auto-namelen magic, we call\n> interpret_nth_prior_checkout(), which gets fed the bogus\n> \"0\". This was broken by 8cd4249c4 (interpret_branch_name:\n> always respect \"namelen\" parameter, 2014-01-15).  Though to\n> be fair to that commit, it was broken in the _opposite_\n> direction before, where we would always treat \"name\" as a\n> string even if a length was passed.\n\nThat commit is mine, by the way. More embarrassing than introducing the\nbug is that I _noticed_ the problem at the time and wrote a paragraph in\nthe commit message rationalizing why it was OK, rather than just doing\nthis trivial fix.\n\n-Peff\n"},{"id":"312765","messageId":"xmqq4lzfxwmq.fsf@gitster.mtv.corp.google.com","threadId":"45234","inReplyTo":"20170227093122.3jdb2b62hlbbio5r@sigill.intra.peff.net","subject":"Re: [PATCH] interpret_branch_name(): handle auto-namelen for @{-1}","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-27T19:27:09Z","receivedAt":"2017-02-27T19:54:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Mon, Feb 27, 2017 at 04:25:40AM -0500, Jeff King wrote:\n>\n>> However, before we do that auto-namelen magic, we call\n>> interpret_nth_prior_checkout(), which gets fed the bogus\n>> \"0\". This was broken by 8cd4249c4 (interpret_branch_name:\n>> always respect \"namelen\" parameter, 2014-01-15).  Though to\n>> be fair to that commit, it was broken in the _opposite_\n>> direction before, where we would always treat \"name\" as a\n>> string even if a length was passed.\n>\n> That commit is mine, by the way. More embarrassing than introducing the\n> bug is that I _noticed_ the problem at the time and wrote a paragraph in\n> the commit message rationalizing why it was OK, rather than just doing\n> this trivial fix.\n\nThanks, I should also be embarrased since I didn't even notice the\nissue when we queued it ;-)\n"}]}