{"thread":{"id":"35896","subject":"Output from \"rev-parse --resolve-git-dir\" changed: two lines (prints original argument)","startedAt":"2014-02-17T08:13:40Z","lastAt":"2014-02-19T09:09:48Z","messageCount":5,"participants":["Daniel Hahler","John Keeping","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"234912","messageId":"5301C4B4.5070503@thequod.de","threadId":"35896","inReplyTo":null,"subject":"Output from \"rev-parse --resolve-git-dir\" changed: two lines (prints original argument)","fromName":"Daniel Hahler","fromEmail":"genml+git-2014@thequod.de","sentAt":"2014-02-17T08:13:40Z","receivedAt":"2014-02-17T08:13:40Z","isPatch":false,"sender":{"key":"genml+git-2014@thequod.de","avatar":null},"body":"I have noticed that the output from \"git rev-parse --resolve-git-dir\" changed.\n\nWhile it used to only print the resolved git dir, it now prints the argument passed to it itself:\n\n % git rev-parse --resolve-git-dir .git\n /path/to/root/.git/modules/vim/bundle/reporoot\n .git\n\nThis is with Git version 1.8.5.3, but also happens with master.\n\nYou can use \"--flags\" to avoid this (\"git rev-parse --flags --resolve-git-dir .git\"), but it is unclear from the documentation if that's the intended beahviour.\n\nIt seems like the \"--resolve-git-dir\" subcommand needs to consume the argument passed to it.\n\n\nRegards,\nDaniel.\n\n-- \nhttp://daniel.hahler.de/\n\n"},{"id":"234913","messageId":"65ea2c8ce4f0d60f0d93f730aac50183ba63bd1c.1392626780.git.john@keeping.me.uk","threadId":"35896","inReplyTo":"5301C4B4.5070503@thequod.de","subject":"[PATCH] rev-parse: fix --resolve-git-dir argument handling","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2014-02-17T08:46:20Z","receivedAt":"2014-02-17T08:46:20Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"There are two problems here:\n\n1) If no argument is provided, then the command segfaults\n2) The argument is not consumed, so there will be excess output\n\nFix both of these in one go by restructuring the handler for this\noption.\n\nReported-by: Daniel Hahler \nSigned-off-by: John Keeping \n---\n builtin/rev-parse.c | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\nindex aaeb611..645cc4a 100644\n--- a/builtin/rev-parse.c\n+++ b/builtin/rev-parse.c\n@@ -738,9 +738,12 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--resolve-git-dir\")) {\n-\t\t\t\tconst char *gitdir = resolve_gitdir(argv[i+1]);\n+\t\t\t\tconst char *gitdir;\n+\t\t\t\tif (++i >= argc)\n+\t\t\t\t\tdie(\"--resolve-git-dir requires an argument\");\n+\t\t\t\tgitdir = resolve_gitdir(argv[i]);\n \t\t\t\tif (!gitdir)\n-\t\t\t\t\tdie(\"not a gitdir '%s'\", argv[i+1]);\n+\t\t\t\t\tdie(\"not a gitdir '%s'\", argv[i]);\n \t\t\t\tputs(gitdir);\n \t\t\t\tcontinue;\n \t\t\t}\n-- \n1.9.0.6.g037df60.dirty\n"},{"id":"235016","messageId":"xmqq61ocdug5.fsf@gitster.dls.corp.google.com","threadId":"35896","inReplyTo":"65ea2c8ce4f0d60f0d93f730aac50183ba63bd1c.1392626780.git.john@keeping.me.uk","subject":"Re: [PATCH] rev-parse: fix --resolve-git-dir argument handling","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-02-18T20:42:02Z","receivedAt":"2014-02-18T20:42:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"John Keeping writes:\n\n> There are two problems here:\n>\n> 1) If no argument is provided, then the command segfaults\n> 2) The argument is not consumed, so there will be excess output\n>\n> Fix both of these in one go by restructuring the handler for this\n> option.\n>\n> Reported-by: Daniel Hahler \n> Signed-off-by: John Keeping \n> ---\n\nLooks sensible; thanks.\n\n> builtin/rev-parse.c | 7 +++++--\n> 1 file changed, 5 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\n> index aaeb611..645cc4a 100644\n> --- a/builtin/rev-parse.c\n> +++ b/builtin/rev-parse.c\n> @@ -738,9 +738,12 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n> \t\t\t\tcontinue;\n> \t\t\t}\n> \t\t\tif (!strcmp(arg, \"--resolve-git-dir\")) {\n> -\t\t\t\tconst char *gitdir = resolve_gitdir(argv[i+1]);\n> +\t\t\t\tconst char *gitdir;\n> +\t\t\t\tif (++i >= argc)\n> +\t\t\t\t\tdie(\"--resolve-git-dir requires an argument\");\n> +\t\t\t\tgitdir = resolve_gitdir(argv[i]);\n> \t\t\t\tif (!gitdir)\n> -\t\t\t\t\tdie(\"not a gitdir '%s'\", argv[i+1]);\n> +\t\t\t\t\tdie(\"not a gitdir '%s'\", argv[i]);\n> \t\t\t\tputs(gitdir);\n> \t\t\t\tcontinue;\n> \t\t\t}\n"},{"id":"235042","messageId":"xmqqzjlo9ce6.fsf@gitster.dls.corp.google.com","threadId":"35896","inReplyTo":"xmqq61ocdug5.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] rev-parse: fix --resolve-git-dir argument handling","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-02-19T00:25:37Z","receivedAt":"2014-02-19T00:25:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> John Keeping writes:\n>\n>> There are two problems here:\n>>\n>> 1) If no argument is provided, then the command segfaults\n>> 2) The argument is not consumed, so there will be excess output\n>>\n>> Fix both of these in one go by restructuring the handler for this\n>> option.\n>>\n>> Reported-by: Daniel Hahler \n>> Signed-off-by: John Keeping \n>> ---\n>\n> Looks sensible; thanks.\n\nEhh, I spoke too fast. Don't we already have this queued as a43219f2\n(rev-parse: check i before using argv[i] against argc, 2014-01-28)?\n"},{"id":"235050","messageId":"20140219090948.GD1048@serenity.lan","threadId":"35896","inReplyTo":"xmqqzjlo9ce6.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] rev-parse: fix --resolve-git-dir argument handling","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2014-02-19T09:09:48Z","receivedAt":"2014-02-19T09:09:48Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Tue, Feb 18, 2014 at 04:25:37PM -0800, Junio C Hamano wrote:\n> Junio C Hamano writes:\n> \n> > John Keeping writes:\n> >\n> >> There are two problems here:\n> >>\n> >> 1) If no argument is provided, then the command segfaults\n> >> 2) The argument is not consumed, so there will be excess output\n> >>\n> >> Fix both of these in one go by restructuring the handler for this\n> >> option.\n> >>\n> >> Reported-by: Daniel Hahler \n> >> Signed-off-by: John Keeping \n> >> ---\n> >\n> > Looks sensible; thanks.\n> \n> Ehh, I spoke too fast. Don't we already have this queued as a43219f2\n> (rev-parse: check i before using argv[i] against argc, 2014-01-28)?\n\nYes, and it catches more cases than mine. I only checked against master\nand had missed that when it went past on the list.\n"}]}