# Output from "rev-parse --resolve-git-dir" changed: two lines (prints original argument) 5 messages from 2014-02-17 to 2014-02-19. Participants: Daniel Hahler, John Keeping, Junio C Hamano. Thread: https://gitlist.dev/t/35896 ## Daniel Hahler, 2014-02-17 08:13 Subject: Output from "rev-parse --resolve-git-dir" changed: two lines (prints original argument) Message-ID: <5301C4B4.5070503@thequod.de> URL: https://gitlist.dev/e/5301C4B4.5070503%40thequod.de ``` I have noticed that the output from "git rev-parse --resolve-git-dir" changed. While it used to only print the resolved git dir, it now prints the argument passed to it itself: % git rev-parse --resolve-git-dir .git /path/to/root/.git/modules/vim/bundle/reporoot .git This is with Git version 1.8.5.3, but also happens with master. You 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. It seems like the "--resolve-git-dir" subcommand needs to consume the argument passed to it. Regards, Daniel. -- http://daniel.hahler.de/ ``` ## John Keeping, 2014-02-17 08:46 Subject: [PATCH] rev-parse: fix --resolve-git-dir argument handling Message-ID: <65ea2c8ce4f0d60f0d93f730aac50183ba63bd1c.1392626780.git.john@keeping.me.uk> URL: https://gitlist.dev/e/65ea2c8ce4f0d60f0d93f730aac50183ba63bd1c.1392626780.git.john%40keeping.me.uk In-Reply-To: <5301C4B4.5070503@thequod.de> ``` There are two problems here: 1) If no argument is provided, then the command segfaults 2) The argument is not consumed, so there will be excess output Fix both of these in one go by restructuring the handler for this option. Reported-by: Daniel Hahler Signed-off-by: John Keeping --- builtin/rev-parse.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c index aaeb611..645cc4a 100644 --- a/builtin/rev-parse.c +++ b/builtin/rev-parse.c @@ -738,9 +738,12 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) continue; } if (!strcmp(arg, "--resolve-git-dir")) { - const char *gitdir = resolve_gitdir(argv[i+1]); + const char *gitdir; + if (++i >= argc) + die("--resolve-git-dir requires an argument"); + gitdir = resolve_gitdir(argv[i]); if (!gitdir) - die("not a gitdir '%s'", argv[i+1]); + die("not a gitdir '%s'", argv[i]); puts(gitdir); continue; } -- 1.9.0.6.g037df60.dirty ``` ## Junio C Hamano, 2014-02-18 20:42 Subject: Re: [PATCH] rev-parse: fix --resolve-git-dir argument handling Message-ID: URL: https://gitlist.dev/e/xmqq61ocdug5.fsf%40gitster.dls.corp.google.com In-Reply-To: <65ea2c8ce4f0d60f0d93f730aac50183ba63bd1c.1392626780.git.john@keeping.me.uk> ``` John Keeping writes: > There are two problems here: > > 1) If no argument is provided, then the command segfaults > 2) The argument is not consumed, so there will be excess output > > Fix both of these in one go by restructuring the handler for this > option. > > Reported-by: Daniel Hahler > Signed-off-by: John Keeping > --- Looks sensible; thanks. > builtin/rev-parse.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c > index aaeb611..645cc4a 100644 > --- a/builtin/rev-parse.c > +++ b/builtin/rev-parse.c > @@ -738,9 +738,12 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) > continue; > } > if (!strcmp(arg, "--resolve-git-dir")) { > - const char *gitdir = resolve_gitdir(argv[i+1]); > + const char *gitdir; > + if (++i >= argc) > + die("--resolve-git-dir requires an argument"); > + gitdir = resolve_gitdir(argv[i]); > if (!gitdir) > - die("not a gitdir '%s'", argv[i+1]); > + die("not a gitdir '%s'", argv[i]); > puts(gitdir); > continue; > } ``` ## Junio C Hamano, 2014-02-19 00:25 Subject: Re: [PATCH] rev-parse: fix --resolve-git-dir argument handling Message-ID: URL: https://gitlist.dev/e/xmqqzjlo9ce6.fsf%40gitster.dls.corp.google.com In-Reply-To: ``` Junio C Hamano writes: > John Keeping writes: > >> There are two problems here: >> >> 1) If no argument is provided, then the command segfaults >> 2) The argument is not consumed, so there will be excess output >> >> Fix both of these in one go by restructuring the handler for this >> option. >> >> Reported-by: Daniel Hahler >> Signed-off-by: John Keeping >> --- > > Looks sensible; thanks. Ehh, I spoke too fast. Don't we already have this queued as a43219f2 (rev-parse: check i before using argv[i] against argc, 2014-01-28)? ``` ## John Keeping, 2014-02-19 09:09 Subject: Re: [PATCH] rev-parse: fix --resolve-git-dir argument handling Message-ID: <20140219090948.GD1048@serenity.lan> URL: https://gitlist.dev/e/20140219090948.GD1048%40serenity.lan In-Reply-To: ``` On Tue, Feb 18, 2014 at 04:25:37PM -0800, Junio C Hamano wrote: > Junio C Hamano writes: > > > John Keeping writes: > > > >> There are two problems here: > >> > >> 1) If no argument is provided, then the command segfaults > >> 2) The argument is not consumed, so there will be excess output > >> > >> Fix both of these in one go by restructuring the handler for this > >> option. > >> > >> Reported-by: Daniel Hahler > >> Signed-off-by: John Keeping > >> --- > > > > Looks sensible; thanks. > > Ehh, I spoke too fast. Don't we already have this queued as a43219f2 > (rev-parse: check i before using argv[i] against argc, 2014-01-28)? Yes, and it catches more cases than mine. I only checked against master and had missed that when it went past on the list. ```