Two problems on alias of git

Two problems on alias of git

[Kana Natsuno]

Hello.  I found 2 problems on alias of git.


The first one is that git crashes with the following situation.
Without GIT_TRACE, everthing works well.  But with GIT_TRACE=1,
git crashes every time.

---- 8< ---- 8< ---- 8< ---- 8< ---- 8< ----
$ git remote show origin
* remote origin
  URL: git://repo.or.cz/git.git
  Remote branch merged with 'git pull' while on branch master
    master
  Tracked remote branches
    html
    maint
    man
    master
    next
    pu
    todo

$ git --version
git version 1.6.2.rc0.90.g0753

$ git show HEAD | grep commit
commit 3536ae331014c68a25c80b3fb530a19c8dee0f11

$ uname -a                   
Darwin foobarbaz 9.6.1 Darwin Kernel Version 9.6.1: Sun Dec 21 19:45:33 PST 2008; root:xnu-1228.9.75~4/RELEASE_I386 i386 i386 MacBook5,2 Darwin

$ git config --get alias.l1
log --pretty=oneline

$ GIT_TRACE=1 git l1         
trace: exec: 'git-l1'
trace: run_command: 'git-l1'
trace: exec 'git-l1' failed: No such file or directory
[1]    41772 segmentation fault  GIT_TRACE=1 git l1
---- >8 ---- >8 ---- >8 ---- >8 ---- >8 ----


The second one is that git doesn't expand a kind of aliases
properly, especially with double quotation marks (").  I used the
following alias recently to list the last 10 commits on HEAD:

[alias]
        lr = !git l1 | head | tac

Then I want to extend this alias to list the last N commits on
a branch with "git lr 20", "git lr master", "git lr master 20",
etc.  So that I wrote the following definition (note that the
actual definition is written in a single line, though the quoted
definition is folded in multiple lines for readability):

[alias]
        lr = !$SHELL -c '
                n=10;
                1="${1:-$n}";
                if ! [ "${1##[0-9]*}" = "" ]; then
                  t="$1";
                  1="${2:-$n}";
                  2="$t";
                fi;
                git --no-pager l1 --reverse -"$1" "${2:-HEAD}"
              ' __dummy__

But it doesn't work because git expands as follows:

$ git config --get alias.lr
!$SHELL -c '1=${1:-10}

Double quotation marks (") are removed and the aliased string is
cut at a random position.  I expect that the aliased string is
passed to system() as-is, but git doesn't so.  Why does git behave
so?  Is it a bug or an intentional behavior?


-- 
To Vim, or not to Vim.
http://whileimautomaton.net/

Re: Two problems on alias of git

[Jakub Narebski]

"Kana Natsuno" <kana@whileimautomaton.net> writes:

> Hello.  I found 2 problems on alias of git.

[...]
> The second one is that git doesn't expand a kind of aliases
> properly, especially with double quotation marks (").  I used the
> following alias recently to list the last 10 commits on HEAD:
> 
> [alias]
>         lr = !git l1 | head | tac
> 
> Then I want to extend this alias to list the last N commits on
> a branch with "git lr 20", "git lr master", "git lr master 20",
> etc.  So that I wrote the following definition (note that the
> actual definition is written in a single line, though the quoted
> definition is folded in multiple lines for readability):
> 
> [alias]
>         lr = !$SHELL -c '
>                 n=10;
>                 1="${1:-$n}";
>                 if ! [ "${1##[0-9]*}" = "" ]; then
>                   t="$1";
>                   1="${2:-$n}";
>                   2="$t";
>                 fi;
>                 git --no-pager l1 --reverse -"$1" "${2:-HEAD}"
>               ' __dummy__
> 
> But it doesn't work because git expands as follows:
> 
> $ git config --get alias.lr
> !$SHELL -c '1=${1:-10}
> 
> Double quotation marks (") are removed and the aliased string is
> cut at a random position.  I expect that the aliased string is
> passed to system() as-is, but git doesn't so.  Why does git behave
> so?  Is it a bug or an intentional behavior?

I don't know if it is a bug or a feature, but git-config supports
quoted strings (required if you want to have value which has trailing
or leading whitespace, or which contains '#' which is beginning of
comment character).  Inside quoted string you need to escape '"':

   [string]
        quotes = "quoted \" string ' with # character"

expands as intended.  Perhaps stripping of double quotes
inside string are artifact of that feature.  Try escaping or
double escaping quotes: \" or \\\".

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: Two problems on alias of git

[Kana Natsuno]

On Tue, 05 May 2009 23:03:49 +0900, Jakub Narebski <jnareb@gmail.com> wrote:
> I don't know if it is a bug or a feature, but git-config supports
> quoted strings (required if you want to have value which has trailing
> or leading whitespace, or which contains '#' which is beginning of
> comment character).  Inside quoted string you need to escape '"':
>
>    [string]
>         quotes = "quoted \" string ' with # character"
>
> expands as intended.  Perhaps stripping of double quotes
> inside string are artifact of that feature.  Try escaping or
> double escaping quotes: \" or \\\".

Thank you for the information.  I've read the source code of git
and I confirmed that this stripping is caused by parse_value() in
config.c.


And as I changed the old definition

        lr = !$SHELL -c 'n=10; 1="${1:-$n}"; ...' __dummy__

by enclosing all text in double quotes and substituting '"' with
'\"' as follows,

	lr = "!$SHELL -c 'n=10; 1=\"${1:-$n}\"; ...' __dummy__"

it works well now.

	$ git config --get alias.lr
	!$SHELL -c 'n=10; 1="${1:-$n}"; if ! [ "${1##[0-9]*}" ...


-- 
To Vim, or not to Vim.
http://whileimautomaton.net/

Re: Two problems on alias of git

[Jakub Narebski]

On Tue, 5 May 2009, Kana Natsuno wrote:
> On Tue, 05 May 2009 23:03:49 +0900, Jakub Narebski <jnareb@gmail.com> wrote:

> > I don't know if it is a bug or a feature, but git-config supports
> > quoted strings (required if you want to have value which has trailing
> > or leading whitespace, or which contains '#' which is beginning of
> > comment character).  Inside quoted string you need to escape '"':

Errr... what I meant here is that I don't know whether stripping
quotes inside value which is not quoted (does not begin and end in '"')
is intended or unintended consequence of git-config behaviour.

> >
> >    [string]
> >         quotes = "quoted \" string ' with # character"
> >
> > expands as intended.  Perhaps stripping of double quotes
> > inside string are artifact of that feature.  Try escaping or
> > double escaping quotes: \" or \\\".
> 
> Thank you for the information.  I've read the source code of git
> and I confirmed that this stripping is caused by parse_value() in
> config.c.
> 
> 
> And as I changed the old definition
> 
>         lr = !$SHELL -c 'n=10; 1="${1:-$n}"; ...' __dummy__
> 
> by enclosing all text in double quotes and substituting '"' with
> '\"' as follows,
> 
> 	lr = "!$SHELL -c 'n=10; 1=\"${1:-$n}\"; ...' __dummy__"
> 
> it works well now.
> 
> 	$ git config --get alias.lr
> 	!$SHELL -c 'n=10; 1="${1:-$n}"; if ! [ "${1##[0-9]*}" ...

By the way, you can use continuation-of-line character (end line
with '\') and/or can embed newlines using C escape sequence, i.e. "\n".

So your code can look like this (although I am not sure it is worth it):

  [alias]
        lr = "!$SHELL -c '                               \n\
                n=10;                                    \n\
                1=\"${1:-$n}\";                          \n\
                if ! [ \"${1##[0-9]*}\" = \"\" ]; then   \n\
                  t=\"$1\";                              \n\
                  1=\"${2:-$n}\";                        \n\
                  2=\"$t\";                              \n\
                fi;                                      \n\
                git --no-pager l1 --reverse -\"$1\" \"${2:-HEAD}\" \n\
              ' __dummy__"

BTW. you need to quote value because it contains comment character '#'
in 4th line of script.

-- 
Jakub Narebski
Poland

Re: Two problems on alias of git

[Kana Natsuno]

On Wed, 06 May 2009 00:42:49 +0900, Jakub Narebski <jnareb@gmail.com> wrote:
> By the way, you can use continuation-of-line character (end line
> with '\') and/or can embed newlines using C escape sequence, i.e. "\n".
>
> So your code can look like this (although I am not sure it is worth it):
>
>   [alias]
>         lr = "!$SHELL -c '                               \n\
>                 n=10;                                    \n\
>                 1=\"${1:-$n}\";                          \n\
>                 if ! [ \"${1##[0-9]*}\" = \"\" ]; then   \n\
>                   t=\"$1\";                              \n\
>                   1=\"${2:-$n}\";                        \n\
>                   2=\"$t\";                              \n\
>                 fi;                                      \n\
>                 git --no-pager l1 --reverse -\"$1\" \"${2:-HEAD}\" \n\
>               ' __dummy__"

Thank you again, I didn't know that.  It is better than what I wrote.


> BTW. you need to quote value because it contains comment character '#'
> in 4th line of script.

Really?  As far as I read the code of git, especially parse_value() in config.c,
it is not necessary to escape '#'s because they are inside of the outermost
doublequotes and they should not be escaped, because \# is an unknown escape
sequence and git rejects them.  If #s are escaped, it causes an error as follows:

$ git config --get alias.lr
fatal: bad config file line 29 in /Users/kana/.gitconfig


-- 
To Vim, or not to Vim.
http://whileimautomaton.net/

Re: Two problems on alias of git

[Jakub Narebski]

On Wed, 5 May 2009 (yes, different timezones), Kana Natsuno wrote:
> On Wed, 06 May 2009 00:42:49 +0900, Jakub Narebski <jnareb@gmail.com> wrote:

> > By the way, you can use continuation-of-line character (end line
> > with '\') and/or can embed newlines using C escape sequence, i.e. "\n".
> >
> > So your code can look like this (although I am not sure it is worth it):
> >
> >   [alias]
> >         lr = "!$SHELL -c '                               \n\
> >                 n=10;                                    \n\
> >                 1=\"${1:-$n}\";                          \n\
> >                 if ! [ \"${1##[0-9]*}\" = \"\" ]; then   \n\
> >                   t=\"$1\";                              \n\
> >                   1=\"${2:-$n}\";                        \n\
> >                   2=\"$t\";                              \n\
> >                 fi;                                      \n\
> >                 git --no-pager l1 --reverse -\"$1\" \"${2:-HEAD}\" \n\
> >               ' __dummy__"
> 
> Thank you again, I didn't know that.  It is better than what I wrote.

By the way, "\n" is there only for nice looking output of git-config
for this variable.  For config file you need only to escape end of line. 
 
> > BTW. you need to quote value because it contains comment character '#'
> > in 4th line of script.
> 
> Really?  As far as I read the code of git, especially parse_value() in config.c,
> it is not necessary to escape '#'s because they are inside of the outermost
> doublequotes and they should not be escaped, because \# is an unknown escape
> sequence and git rejects them.  If #s are escaped, it causes an error as follows:
> 
> $ git config --get alias.lr
> fatal: bad config file line 29 in /Users/kana/.gitconfig

Errrr... what I meant is that you have to _quote_ value if it contains
comment character, compare

  [string]
  	unquoted =  a # b
  	quoted   = "a # b"

not that you have to escape comment character '#'. But I forgot about
the fact that ## in your example is inside quotes anyway.

-- 
Jakub Narebski
Poland

Re: Two problems on alias of git

[Jeff King]

On Tue, May 05, 2009 at 09:42:04PM +0900, Kana Natsuno wrote:

> The first one is that git crashes with the following situation.
> Without GIT_TRACE, everthing works well.  But with GIT_TRACE=1,
> git crashes every time.

I wasn't able to reproduce on Linux, but valgrind found it. The patch
below should fix it.

-- >8 --
Subject: [PATCH] fix GIT_TRACE segfault with shell-quoted aliases

The alias argv comes from the split_cmdline function, which
splits the config text for the alias into an array of
strings. It returns the number of elements in the array, but
does not actually put a NULL at the end of the array.
Later, the trace function tries to print this argv and
assumes that it has the trailing NULL.

The split_cmdline function is probably at fault, since argv
lists almost always end with a NULL signal. This patch adds
one, in addition to the returned count; this doesn't hurt
the other callers at all, since they were presumably using
the count already (and will never look at the NULL).

While we're there and using ALLOC_GROW, let's clean up the
other manual grow.

Signed-off-by: Jeff King <peff@peff.net>
---
 alias.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/alias.c b/alias.c
index e687fe5..372b7d8 100644
--- a/alias.c
+++ b/alias.c
@@ -38,10 +38,7 @@ int split_cmdline(char *cmdline, const char ***argv)
 			while (cmdline[++src]
 					&& isspace(cmdline[src]))
 				; /* skip */
-			if (count >= size) {
-				size += 16;
-				*argv = xrealloc(*argv, sizeof(char *) * size);
-			}
+			ALLOC_GROW(*argv, count+1, size);
 			(*argv)[count++] = cmdline + dst;
 		} else if (!quoted && (c == '\'' || c == '"')) {
 			quoted = c;
@@ -72,6 +69,9 @@ int split_cmdline(char *cmdline, const char ***argv)
 		return error("unclosed quote");
 	}
 
+	ALLOC_GROW(*argv, count+1, size);
+	(*argv)[count] = NULL;
+
 	return count;
 }
 
-- 
1.6.3.203.g7c7de.dirty