threads / discuss / 35896

Output from "rev-parse --resolve-git-dir" changed: two lines (prints original argument)

Subject: Output from "rev-parse --resolve-git-dir" changed: two lines (prints original argument)

## tl;dr

5 messages between Feb 17, 2014 and Feb 19, 2014.

replies: 4people: 3as markdown or json

Daniel Hahler· Feb 17, 2014, 08:13 UTC · lore

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· Feb 17, 2014, 08:46 UTC · re: Daniel Hahler · lore

[PATCH] rev-parse: fix --resolve-git-dir argument handling

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 <genml+git-2014@thequod.de>
Signed-off-by: John Keeping <john@keeping.me.uk>
---
 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· Feb 18, 2014, 20:42 UTC · re: John Keeping · lore

Re: [PATCH] rev-parse: fix --resolve-git-dir argument handling

John Keeping <john@keeping.me.uk> writes:

Show 11 quoted lines

> 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 <genml+git-2014@thequod.de>
> Signed-off-by: John Keeping <john@keeping.me.uk>
> ---

Looks sensible; thanks.

Show 22 quoted lines

>  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· Feb 19, 2014, 00:25 UTC · re: Junio C Hamano · lore

Re: [PATCH] rev-parse: fix --resolve-git-dir argument handling

Junio C Hamano <gitster@pobox.com> writes:

Show 15 quoted lines

> John Keeping <john@keeping.me.uk> 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 <genml+git-2014@thequod.de>
>> Signed-off-by: John Keeping <john@keeping.me.uk>
>> ---
>
> 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· Feb 19, 2014, 09:09 UTC · re: Junio C Hamano · lore

Re: [PATCH] rev-parse: fix --resolve-git-dir argument handling

On Tue, Feb 18, 2014 at 04:25:37PM -0800, Junio C Hamano wrote:

Show 20 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > John Keeping <john@keeping.me.uk> 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 <genml+git-2014@thequod.de>
> >> Signed-off-by: John Keeping <john@keeping.me.uk>
> >> ---
> >
> > 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.