Re: [bug] generic issue with git_config handlers

[Christian Couder <chriscool@tuxfamily.org>]

Le jeudi 31 janvier 2008, Pierre Habouzit a écrit :
> On Thu, Jan 31, 2008 at 09:25:32AM +0000, Junio C Hamano wrote:
> > Pierre Habouzit <madcoder@debian.org> writes:
> > >   One of my co-workers stumbled upon a misfeature of the git config
> > > parser. The following syntax is allowed:
> > >
> > >     [section]
> > > 	 foo
> >
> > Yeah, that is how "truth" value of boolean is spelled.
> >
> > > [user]
> > >     name
> >
> > That's very unfortunate.  Whatever is expecting string value
> > should check for NULL.  Fix should probably be easy enough for
> > any git-hacker-wannabe to tackle ;-)
>
>   I think so too, though my count is something like 40 functions to
> investigate (the 40 handlers) and where it recurses into ;) Too much
> work for the time I have right now.

I would suggest this patch:

---8<---
diff --git a/config.c b/config.c
index 526a3f4..92613c5 100644
--- a/config.c
+++ b/config.c
@@ -139,7 +139,7 @@ static int get_value(config_fn_t fn, char *name, 
unsigned in
                if (!value)
                        return -1;
        }
-       return fn(name, value);
+       return fn(name, value ? value : "");
 }

 static int get_extended_base_var(char *name, int baselen, int c)
---8<---

but it breaks some test cases.

$ ./t1300-repo-config.sh -d -i -v

[...]

* expecting success: git config --get-regexp novalue > output &&
         cmp output expect
output expect differ: char 17, line 1
* FAIL 34: get-regexp variable with no value
        git config --get-regexp novalue > output &&
                 cmp output expect

$ cat output | hexdump -C
00000000  6e 6f 76 61 6c 75 65 2e  76 61 72 69 61 62 6c 65  |
novalue.variable|
00000010  20 0a                                             | .|
00000012

$ cat expect | hexdump -C
00000000  6e 6f 76 61 6c 75 65 2e  76 61 72 69 61 62 6c 65  |
novalue.variable|
00000010  0a                                                |.|
00000011

I don't know if the added space is a big problem.

It comes from the following code in builtin-config.c:44

	if (show_keys) {
		if (value_)
			printf("%s%c", key_, key_delim);
		else
			printf("%s", key_);

where "value_" is now "" instead of NULL.

At this point, as I don't know much the code in these files, I think I could 
very well use some advice from people more familiar with this.

Thanks in advance,
Christian.