{"thread":{"id":"21121","subject":"[PATCH] Reserve a slot for argv[0] in default_arg.","startedAt":"2009-10-03T13:29:31Z","lastAt":"2009-10-05T18:45:32Z","messageCount":10,"participants":["Petter Urkedal","Jeff King","Stephen Boyd","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"124174","messageId":"1254576571-29274-1-git-send-email-urkedal@nbi.dk","threadId":"21121","inReplyTo":null,"subject":"[PATCH] Reserve a slot for argv[0] in default_arg.","fromName":"Petter Urkedal","fromEmail":"urkedal@nbi.dk","sentAt":"2009-10-03T13:29:31Z","receivedAt":"2009-10-03T13:29:31Z","isPatch":true,"sender":{"key":"urkedal@nbi.dk","avatar":null},"body":"Setting \"av\" to one slot before the allocated \"default_arg\" array causes\nglibc abort with \"free(): invalid next size (normal)\" in some\nconfigurations (Gentoo, glibc-2.9_p20081201-r2, gcc-5.3.2 with PIE).\n---\n builtin-show-branch.c |    7 +++++--\n 1 files changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-show-branch.c b/builtin-show-branch.c\nindex 3510a86..3ab72b7 100644\n--- a/builtin-show-branch.c\n+++ b/builtin-show-branch.c\n@@ -568,6 +568,9 @@ static int git_show_branch_config(const char *var, const char *value, void *cb)\n \t\tif (default_alloc <= default_num + 1) {\n \t\t\tdefault_alloc = default_alloc * 3 / 2 + 20;\n \t\t\tdefault_arg = xrealloc(default_arg, sizeof *default_arg * default_alloc);\n+\t\t\tif (!default_num)\n+\t\t\t    /* One unused position for argv[0]. */\n+\t\t\t    default_arg[default_num++] = NULL;\n \t\t}\n \t\tdefault_arg[default_num++] = xstrdup(value);\n \t\tdefault_arg[default_num] = NULL;\n@@ -692,8 +695,8 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \n \t/* If nothing is specified, try the default first */\n \tif (ac == 1 && default_num) {\n-\t\tac = default_num + 1;\n-\t\tav = default_arg - 1; /* ick; we would not address av[0] */\n+\t\tac = default_num;\n+\t\tav = default_arg;\n \t}\n \n \tac = parse_options(ac, av, prefix, builtin_show_branch_options,\n-- \n1.6.4.4\n"},{"id":"299134","messageId":"20091004133333.GA13894@sigill.intra.peff.net","threadId":"21121","inReplyTo":"1254576571-29274-1-git-send-email-urkedal@nbi.dk","subject":"Re: [PATCH] Reserve a slot for argv[0] in default_arg.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-10-04T13:33:33Z","receivedAt":"2009-10-04T13:33:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Oct 03, 2009 at 03:29:31PM +0200, Petter Urkedal wrote:\n\n> Setting \"av\" to one slot before the allocated \"default_arg\" array causes\n> glibc abort with \"free(): invalid next size (normal)\" in some\n> configurations (Gentoo, glibc-2.9_p20081201-r2, gcc-5.3.2 with PIE).\n\nThanks, your fix looks sane. But I am curious about whether we are\ntriggering some glibc pickiness that is in your setup, or if we are\nsomehow violating the assumption that we only ever look at\ndefault_arg[1] and beyond.\n\nWhat show-branch command did you issue to hit this? I was hoping to run\nit under valgrind.\n\nAlso:\n\n> +\t\t\tif (!default_num)\n> +\t\t\t    /* One unused position for argv[0]. */\n> +\t\t\t    default_arg[default_num++] = NULL;\n\nI don't know if we have a style rule for comments on single line\nconditionals, but I had to read this a few times to make sure it wasn't\nmissing braces.\n\n> -\t\tac = default_num + 1;\n> -\t\tav = default_arg - 1; /* ick; we would not address av[0] */\n> +\t\tac = default_num;\n> +\t\tav = default_arg;\n\nAny time you can remove a comment with \"ick\" in it is probably a good\nthing.  ;)\n\n-Peff\n"},{"id":"299135","messageId":"20091004141355.GA15783@eideticdew.org","threadId":"21121","inReplyTo":"20091004133333.GA13894@sigill.intra.peff.net","subject":"Re: [PATCH] Reserve a slot for argv[0] in default_arg.","fromName":"Petter Urkedal","fromEmail":"urkedal@nbi.dk","sentAt":"2009-10-04T14:13:55Z","receivedAt":"2009-10-04T14:13:55Z","isPatch":true,"sender":{"key":"urkedal@nbi.dk","avatar":null},"body":"On 2009-10-04, Jeff King wrote:\n> On Sat, Oct 03, 2009 at 03:29:31PM +0200, Petter Urkedal wrote:\n> \n> > Setting \"av\" to one slot before the allocated \"default_arg\" array causes\n> > glibc abort with \"free(): invalid next size (normal)\" in some\n> > configurations (Gentoo, glibc-2.9_p20081201-r2, gcc-5.3.2 with PIE).\n> \n> Thanks, your fix looks sane. But I am curious about whether we are\n> triggering some glibc pickiness that is in your setup, or if we are\n> somehow violating the assumption that we only ever look at\n> default_arg[1] and beyond.\n\nI was wondering myself.  I tried to switch off optimisation, but that\nhad no effect.  I'm suspecting PIE, but it could be some other\nconfiguration implied by the Gentoo \"hardened\" use-flag.\n\n> What show-branch command did you issue to hit this? I was hoping to run\n> it under valgrind.\n\nI can reproduce it on my machine with\n\n    mkdir test-repo; cd test-repo\n    /path/to/git init\n    /path/to/git config showbranch.default --topo-order\n    /path/to/git show-branch\n\n> Also:\n> \n> > +\t\t\tif (!default_num)\n> > +\t\t\t    /* One unused position for argv[0]. */\n> > +\t\t\t    default_arg[default_num++] = NULL;\n> \n> I don't know if we have a style rule for comments on single line\n> conditionals, but I had to read this a few times to make sure it wasn't\n> missing braces.\n\nComment's are treated as whitespace, but I'll adjust it for readability.\nMaybe worse: I missed the 8-column indentation.  So, here is the patch\nagain (attached, I hope Git can extract it).\n\n\n1552 eideticdew /tmp/test-repo$ gdb /home/urkedal/proj-ext/git/git show-branch\nGNU gdb 6.8\nCopyright (C) 2008 Free Software Foundation, Inc.\nLicense GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>\nThis is free software: you are free to change and redistribute it.\nThere is NO WARRANTY, to the extent permitted by law.  Type \"show copying\"\nand \"show warranty\" for details.\nThis GDB was configured as \"x86_64-pc-linux-gnu\"...\n/tmp/test-repo/show-branch: No such file or directory.\n(gdb) run show-branch\n[Thread debugging using libthread_db enabled]\n*** glibc detected *** /home/urkedal/proj-ext/git/git: corrupted double-linked list: 0x0000000000734340 ***\n[New Thread 0x7f937d4c36f0 (LWP 15739)]\n======= Backtrace: =========\n/lib/libc.so.6[0x7f937c84e1f2]\n/lib/libc.so.6[0x7f937c84e4a4]\n/lib/libc.so.6[0x7f937c84f7fc]\n/lib/libc.so.6(cfree+0x75)[0x7f937c84fb13]\n/lib/libc.so.6[0x7f937c8407bf]\n/home/urkedal/proj-ext/git/git[0x48c921]\n/home/urkedal/proj-ext/git/git[0x48e617]\n/home/urkedal/proj-ext/git/git[0x44e1d2]\n/home/urkedal/proj-ext/git/git[0x4042a3]\n/home/urkedal/proj-ext/git/git[0x40444d]\n/lib/libc.so.6(__libc_start_main+0xe6)[0x7f937c7fd56e]\n/home/urkedal/proj-ext/git/git[0x403d59]\n======= Memory map: ========\n00400000-004e8000 r-xp 00000000 fd:04 12700046                           /home/urkedal/proj-ext/git/git\n006e7000-006e8000 r--p 000e7000 fd:04 12700046                           /home/urkedal/proj-ext/git/git\n006e8000-006ed000 rw-p 000e8000 fd:04 12700046                           /home/urkedal/proj-ext/git/git\n006ed000-00755000 rw-p 00000000 00:00 0                                  [heap]\n7f9378000000-7f9378021000 rw-p 00000000 00:00 0 \n7f9378021000-7f937c000000 ---p 00000000 00:00 0 \n7f937c3c5000-7f937c3da000 r-xp 00000000 08:01 32689                      /lib64/libgcc_s.so.1\n7f937c3da000-7f937c5d9000 ---p 00015000 08:01 32689                      /lib64/libgcc_s.so.1\n7f937c5d9000-7f937c5da000 r--p 00014000 08:01 32689                      /lib64/libgcc_s.so.1\n7f937c5da000-7f937c5db000 rw-p 00015000 08:01 32689                      /lib64/libgcc_s.so.1\n7f937c5db000-7f937c5dd000 r-xp 00000000 08:01 33045                      /lib64/libdl-2.9.so\n7f937c5dd000-7f937c7dd000 ---p 00002000 08:01 33045                      /lib64/libdl-2.9.so\n7f937c7dd000-7f937c7de000 r--p 00002000 08:01 33045                      /lib64/libdl-2.9.so\n7f937c7de000-7f937c7df000 rw-p 00003000 08:01 33045                      /lib64/libdl-2.9.so\n7f937c7df000-7f937c91e000 r-xp 00000000 08:01 32884                      /lib64/libc-2.9.so\n7f937c91e000-7f937cb1e000 ---p 0013f000 08:01 32884                      /lib64/libc-2.9.so\n7f937cb1e000-7f937cb22000 r--p 0013f000 08:01 32884                      /lib64/libc-2.9.so\n7f937cb22000-7f937cb23000 rw-p 00143000 08:01 32884                      /lib64/libc-2.9.so\n7f937cb23000-7f937cb28000 rw-p 00000000 00:00 0 \n7f937cb28000-7f937cb3d000 r-xp 00000000 08:01 32902                      /lib64/libpthread-2.9.so\n7f937cb3d000-7f937cd3d000 ---p 00015000 08:01 32902                      /lib64/libpthread-2.9.so\n7f937cd3d000-7f937cd3e000 r--p 00015000 08:01 32902                      /lib64/libpthread-2.9.so\n7f937cd3e000-7f937cd3f000 rw-p 00016000 08:01 32902                      /lib64/libpthread-2.9.so\n7f937cd3f000-7f937cd43000 rw-p 00000000 00:00 0 \n7f937cd43000-7f937ce95000 r-xp 00000000 fd:00 1181123                    /usr/lib64/libcrypto.so.0.9.8\n7f937ce95000-7f937d095000 ---p 00152000 fd:00 1181123                    /usr/lib64/libcrypto.so.0.9.8\n7f937d095000-7f937d0a3000 r--p 00152000 fd:00 1181123                    /usr/lib64/libcrypto.so.0.9.8\n7f937d0a3000-7f937d0bb000 rw-p 00160000 fd:00 1181123                    /usr/lib64/libcrypto.so.0.9.8\n7f937d0bb000-7f937d0bf000 rw-p 00000000 00:00 0 \n7f937d0bf000-7f937d0d3000 r-xp 00000000 08:01 32655                      /lib64/libz.so.1.2.3\n7f937d0d3000-7f937d2d2000 ---p 00014000 08:01 32655                      /lib64/libz.so.1.2.3\n7f937d2d2000-7f937d2d3000 r--p 00013000 08:01 32655                      /lib64/libz.so.1.2.3\n7f937d2d3000-7f937d2d4000 rw-p 00014000 08:01 32655                      /lib64/libz.so.1.2.3\n7f937d2d4000-7f937d2f0000 r-xp 00000000 08:01 33042                      /lib64/ld-2.9.so\n7f937d4c3000-7f937d4c6000 rw-p 00000000 00:00 0 \n7f937d4ed000-7f937d4ef000 rw-p 00000000 00:00 0 \n7f937d4ef000-7f937d4f0000 r--p 0001b000 08:01 33042                      /lib64/ld-2.9.so\n7f937d4f0000-7f937d4f1000 rw-p 0001c000 08:01 33042                      /lib64/ld-2.9.so\n7fff94a38000-7fff94a4e000 rw-p 00000000 00:00 0                          [stack]\n7fff94bff000-7fff94c00000 r-xp 00000000 00:00 0                          [vdso]\nffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0                  [vsyscall]\n\nProgram received signal SIGABRT, Aborted.\n[Switching to Thread 0x7f937d4c36f0 (LWP 15739)]\n0x00007f937c810536 in raise () from /lib/libc.so.6\n(gdb) bt\n#0  0x00007f937c810536 in raise () from /lib/libc.so.6\n#1  0x00007f937c811723 in abort () from /lib/libc.so.6\n#2  0x00007f937c84953f in ?? () from /lib/libc.so.6\n#3  0x00007f937c84e1f2 in ?? () from /lib/libc.so.6\n#4  0x00007f937c84e4a4 in ?? () from /lib/libc.so.6\n#5  0x00007f937c84f7fc in ?? () from /lib/libc.so.6\n#6  0x00007f937c84fb13 in free () from /lib/libc.so.6\n#7  0x00007f937c8407bf in ?? () from /lib/libc.so.6\n#8  0x000000000048c921 in get_packed_refs () at refs.c:234\n#9  0x000000000048e617 in do_for_each_ref (base=0x3d7b <Address 0x3d7b out of bounds>, fn=0x3d7b, \n    trim=6, flags=-1, cb_data=0x7f937c8e71e0) at refs.c:598\n#10 0x000000000044e1d2 in cmd_show_branch (ac=<value optimized out>, av=0x734348, prefix=0x0)\n    at builtin-show-branch.c:478\n#11 0x00000000004042a3 in handle_internal_command (argc=1, argv=0x7fff94a4b1e0) at git.c:249\n#12 0x000000000040444d in main (argc=1, argv=0x7fff94a4b1e0) at git.c:436\n(gdb) q\n\n\nFrom a6bea57b5f9e3ebca38afce7829922ccb8f7d24f Mon Sep 17 00:00:00 2001\nFrom: Petter Urkedal <urkedal@nbi.dk>\nDate: Sat, 3 Oct 2009 14:52:41 +0200\nSubject: [PATCH] Reserve a slot for argv[0] in default_arg.\n\nSetting \"av\" to one slot before the allocated \"default_arg\" array causes\nglibc abort with \"free(): invalid next size (normal)\" in some\nconfigurations (Gentoo, glibc-2.9_p20081201-r2, gcc-5.3.2 with PIE).\n---\n builtin-show-branch.c |    8 ++++++--\n 1 files changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-show-branch.c b/builtin-show-branch.c\nindex 3510a86..81c477c 100644\n--- a/builtin-show-branch.c\n+++ b/builtin-show-branch.c\n@@ -568,6 +568,10 @@ static int git_show_branch_config(const char *var, const char *value, void *cb)\n \t\tif (default_alloc <= default_num + 1) {\n \t\t\tdefault_alloc = default_alloc * 3 / 2 + 20;\n \t\t\tdefault_arg = xrealloc(default_arg, sizeof *default_arg * default_alloc);\n+\t\t\tif (!default_num) {\n+\t\t\t\t/* One unused position for argv[0]. */\n+\t\t\t\tdefault_arg[default_num++] = NULL;\n+\t\t\t}\n \t\t}\n \t\tdefault_arg[default_num++] = xstrdup(value);\n \t\tdefault_arg[default_num] = NULL;\n@@ -692,8 +696,8 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \n \t/* If nothing is specified, try the default first */\n \tif (ac == 1 && default_num) {\n-\t\tac = default_num + 1;\n-\t\tav = default_arg - 1; /* ick; we would not address av[0] */\n+\t\tac = default_num;\n+\t\tav = default_arg;\n \t}\n \n \tac = parse_options(ac, av, prefix, builtin_show_branch_options,\n-- \n1.6.4.4\n\n"},{"id":"299136","messageId":"20091004145141.GA16477@eideticdew.org","threadId":"21121","inReplyTo":"20091004133333.GA13894@sigill.intra.peff.net","subject":"Re: [PATCH] Reserve a slot for argv[0] in default_arg.","fromName":"Petter Urkedal","fromEmail":"urkedal@nbi.dk","sentAt":"2009-10-04T14:51:41Z","receivedAt":"2009-10-04T14:51:41Z","isPatch":true,"sender":{"key":"urkedal@nbi.dk","avatar":null},"body":"On 2009-10-04, Jeff King wrote:\n> Thanks, your fix looks sane. But I am curious about whether we are\n> triggering some glibc pickiness that is in your setup, or if we are\n> somehow violating the assumption that we only ever look at\n> default_arg[1] and beyond.\n\nI had a look at parse-options.c.  In parse_options_start, argv is\nassigned to ctx->out, which is overwritten from index 0 in\nparse_options_end.  This will show the problem:\n\ndiff --git a/builtin-show-branch.c b/builtin-show-branch.c\nindex 3510a86..1c587ad 100644\n--- a/builtin-show-branch.c\n+++ b/builtin-show-branch.c\n@@ -691,6 +691,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n                showbranch_use_color = git_use_color_default;\n \n        /* If nothing is specified, try the default first */\n+       printf(\"default_arg = %p\\n\", default_arg);\n        if (ac == 1 && default_num) {\n                ac = default_num + 1;\n                av = default_arg - 1; /* ick; we would not address av[0] */\ndiff --git a/parse-options.c b/parse-options.c\nindex f559411..267e752 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -435,6 +435,7 @@ unknown:\n \n int parse_options_end(struct parse_opt_ctx_t *ctx)\n {\n+       printf(\"Assigning to %p\\n\", ctx->out + ctx->cpidx);\n        memmove(ctx->out + ctx->cpidx, ctx->argv, ctx->argc * sizeof(*ctx->out));\n        ctx->out[ctx->cpidx + ctx->argc] = NULL;\n        return ctx->cpidx + ctx->argc;\n\n"},{"id":"124202","messageId":"20091004182746.GA22995@coredump.intra.peff.net","threadId":"21121","inReplyTo":"20091004141355.GA15783@eideticdew.org","subject":"Re: [PATCH] Reserve a slot for argv[0] in default_arg.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-10-04T18:27:46Z","receivedAt":"2009-10-04T18:27:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Oct 04, 2009 at 04:13:55PM +0200, Petter Urkedal wrote:\n\n> I was wondering myself.  I tried to switch off optimisation, but that\n> had no effect.  I'm suspecting PIE, but it could be some other\n> configuration implied by the Gentoo \"hardened\" use-flag.\n\nNope, it's just a plain old git bug...\n\n> I can reproduce it on my machine with\n> \n>     mkdir test-repo; cd test-repo\n>     /path/to/git init\n>     /path/to/git config showbranch.default --topo-order\n>     /path/to/git show-branch\n\nAh, thanks, for some reason I wasn't able to produce it before, but I\ncan easily replicate it here. I think it's a regression from converting\nshow-branch to use parse_options, which happened in May, but I didn't\nactually bisect it. I'm not sure showbranch.default has worked at all\nsince then (which I guess goes to show how many people are actually\nusing it).\n\nSo your fix is definitely right, and the test case below (which can be\nsquashed in) fails reliably without it.\n\nt3202 is maybe a bit of weird place to put it, but we don't seem to test\nshow-branch anywhere else. It could probably use a \"check that\nshow-branch works at all\" set of tests, but I am not volunteering to\nwrite such a thing. I have always found its output to be one step above\nline noise.\n\nI also looked at putting it in t1200-tutorial.sh near the show-branch\ncall, but that script is an utter mess. Most of the tests don't actually\ncheck the exit status of commands, and there is a random \"test_done\"\nhalfway through the script which skips all of the later tests (including\nthe show-branch test!). Removing that to enable the later tests reveals\nthat they are broken, with such obviously non-working crap as\n\n  git merge -s \"Merge upstream changes.\" master\n\nwhich is clearly bogus. I wonder if we should just remove that script\naltogether; at best it just seems redundant with other tests, and it is\nfull of obvious errors.\n\n> Comment's are treated as whitespace, but I'll adjust it for readability.\n> Maybe worse: I missed the 8-column indentation.  So, here is the patch\n> again (attached, I hope Git can extract it).\n\nThanks, that looks better (I actually didn't even notice the indent\nproblem the first time, but yes, it should be 8 columns).\n\nSquashable test case is below.\n\n-Peff\n\n---\ndiff --git a/t/t3202-show-branch-octopus.sh b/t/t3202-show-branch-octopus.sh\nindex 7fe4a6e..0a5d5e6 100755\n--- a/t/t3202-show-branch-octopus.sh\n+++ b/t/t3202-show-branch-octopus.sh\n@@ -56,4 +56,12 @@ test_expect_success 'show-branch with more than 8 branches' '\n \n '\n \n+test_expect_success 'show-branch with showbranch.default' '\n+\tfor i in $numbers; do\n+\t\tgit config --add showbranch.default branch$i\n+\tdone &&\n+\tgit show-branch >out &&\n+\ttest_cmp expect out\n+'\n+\n test_done\n"},{"id":"124206","messageId":"1254686541.507.9.camel@swboyd-laptop","threadId":"21121","inReplyTo":"20091004182746.GA22995@coredump.intra.peff.net","subject":"Re: [PATCH] Reserve a slot for argv[0] in default_arg.","fromName":"Stephen Boyd","fromEmail":"bebarino@gmail.com","sentAt":"2009-10-04T20:02:21Z","receivedAt":"2009-10-04T20:02:21Z","isPatch":true,"sender":{"key":"bebarino@gmail.com","avatar":"https://avatars.githubusercontent.com/u/38832?v=4"},"body":"On Sun, 2009-10-04 at 14:27 -0400, Jeff King wrote:\n> \n> Ah, thanks, for some reason I wasn't able to produce it before, but I\n> can easily replicate it here. I think it's a regression from converting\n> show-branch to use parse_options, which happened in May, but I didn't\n> actually bisect it. I'm not sure showbranch.default has worked at all\n> since then (which I guess goes to show how many people are actually\n> using it).\n\nCorrect. Junio sent a patch to fix this problem in June[1]. I guess he\nmust have dropped his own patch, or he wasn't satisfied with how parse\noptions clobbers things.\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/121142\n"},{"id":"124210","messageId":"7v4oqen6my.fsf@alter.siamese.dyndns.org","threadId":"21121","inReplyTo":"20091004182746.GA22995@coredump.intra.peff.net","subject":"Re: [PATCH] Reserve a slot for argv[0] in default_arg.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-10-04T22:20:21Z","receivedAt":"2009-10-04T22:20:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stephen Boyd <bebarino@gmail.com> writes:\n\n> On Sun, 2009-10-04 at 14:27 -0400, Jeff King wrote:\n>> \n>> Ah, thanks, for some reason I wasn't able to produce it before, but I\n>> can easily replicate it here. I think it's a regression from converting\n>> show-branch to use parse_options, which happened in May, but I didn't\n>> actually bisect it. I'm not sure showbranch.default has worked at all\n>> since then (which I guess goes to show how many people are actually\n>> using it).\n\nIt is a command specific aliasing mechanism; not even I use the feature\nthese days, since \"alias.*\" is much easier to use.  But there is no strong\nneed to remove it either; it is not too much hassle to keep it for people\nwho do use it.  Perhaps deprecate it and remove it in the long run?\n\n> Correct. Junio sent a patch to fix this problem in June[1]. I guess he\n> must have dropped his own patch, or he wasn't satisfied with how parse\n> options clobbers things.\n>\n> [1] http://article.gmane.org/gmane.comp.version-control.git/121142\n\nI had it kept still in my Inbox; thanks for noticing.  Petter's patch does\nessentially the same thing, but the old patch had a better log message\nthat described where in the history the fix should apply, so I'd probably\nuse that with your test squashed in.\n\nThanks.\n"},{"id":"124224","messageId":"20091005063649.GA25040@eideticdew.org","threadId":"21121","inReplyTo":"7v4oqen6my.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Reserve a slot for argv[0] in default_arg.","fromName":"Petter Urkedal","fromEmail":"urkedal@nbi.dk","sentAt":"2009-10-05T06:36:49Z","receivedAt":"2009-10-05T06:36:49Z","isPatch":true,"sender":{"key":"urkedal@nbi.dk","avatar":null},"body":"On 2009-10-04, Junio C Hamano wrote:\n> It is a command specific aliasing mechanism; not even I use the feature\n> these days, since \"alias.*\" is much easier to use.  But there is no strong\n> need to remove it either; it is not too much hassle to keep it for people\n> who do use it.  Perhaps deprecate it and remove it in the long run?\n\nI didn't know about alias.*.  Excellent.  I'll be using that.\n \n> > Correct. Junio sent a patch to fix this problem in June[1]. I guess he\n> > must have dropped his own patch, or he wasn't satisfied with how parse\n> > options clobbers things.\n> >\n> > [1] http://article.gmane.org/gmane.comp.version-control.git/121142\n> \n> I had it kept still in my Inbox; thanks for noticing.  Petter's patch does\n> essentially the same thing, but the old patch had a better log message\n> that described where in the history the fix should apply, so I'd probably\n> use that with your test squashed in.\n\nThe code is slightly nicer to, I think, but you can probably drop \"+ 20\"\nin the grow-case now.\n"},{"id":"124225","messageId":"20091005064404.GB25040@eideticdew.org","threadId":"21121","inReplyTo":"20091004182746.GA22995@coredump.intra.peff.net","subject":"Re: [PATCH] Reserve a slot for argv[0] in default_arg.","fromName":"Petter Urkedal","fromEmail":"urkedal@nbi.dk","sentAt":"2009-10-05T06:44:04Z","receivedAt":"2009-10-05T06:44:04Z","isPatch":true,"sender":{"key":"urkedal@nbi.dk","avatar":null},"body":"On 2009-10-04, Jeff King wrote:\n> t3202 is maybe a bit of weird place to put it, but we don't seem to test\n> show-branch anywhere else. It could probably use a \"check that\n> show-branch works at all\" set of tests, but I am not volunteering to\n> write such a thing.\n\nLooks good to me.  I'm not so familiar with the source code as I looked\nat it first time when I submitted the patch.\n\n> I have always found its output to be one step above\n> line noise.\n\nI agree show-branch is nosy, and the actual options I was adding was\n\"--topo-order master t/*\" to show only topic branches.  After rebasing\nthem against master, show-branch with these arguments gives a nice\noverview.  Thanks Junio's tip I now have\n\n    alias.show-topics = show-branch --topo-order master t/*\n"},{"id":"124263","messageId":"20091005184532.GA20078@coredump.intra.peff.net","threadId":"21121","inReplyTo":"20091005063649.GA25040@eideticdew.org","subject":"Re: [PATCH] Reserve a slot for argv[0] in default_arg.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-10-05T18:45:32Z","receivedAt":"2009-10-05T18:45:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Oct 05, 2009 at 08:36:49AM +0200, Petter Urkedal wrote:\n\n> On 2009-10-04, Junio C Hamano wrote:\n> > It is a command specific aliasing mechanism; not even I use the feature\n> > these days, since \"alias.*\" is much easier to use.  But there is no strong\n> > need to remove it either; it is not too much hassle to keep it for people\n> > who do use it.  Perhaps deprecate it and remove it in the long run?\n> \n> I didn't know about alias.*.  Excellent.  I'll be using that.\n\nYeah, showbranch.default really seems pointless now. Especially\nconfusing is the fact that it doesn't do whitespace-splitting, so you\ncan't do:\n\n  git config showbranch.default \"--topo-order branch1 branch2\"\n\nbut instead have to set multiple config variables.\n\nI think deprecation makes sense, but I am in no hurry to get rid of it.\nI mainly just wouldn't want people to think it was a useful thing to\nlearn. :)\n\n> The code is slightly nicer to, I think, but you can probably drop \"+ 20\"\n> in the grow-case now.\n\nI think it could actually just be switched to use ALLOC_GROW.\n\n-Peff\n"}]}