From ed838e661551958076beda19c9445101da2b70ae Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Tue, 23 Oct 2012 14:59:26 -0400
Subject: t1300: style updates

The t1300 test script is quite old, and does not use our
modern techniques or styles. This patch updates it in the
following ways:

  1. Use test_cmp instead of cmp (to make failures easier to
     debug).

  2. Use test_cmp instead of 'test $(command) = expected'.
     This makes failures much easier to debug, and also
     makes sure that $(command) exits appropriately.

  3. Use test_must_fail (easier to read, and checks more
     rigorously for signal death).

  4. Write tests with the usual style of:

       test_expect_success 'test name' '
               test commands &&
	       ...
       '

     rather than one-liners, or using backslash-continuation.
     This is purely a style fixup.

There are still a few command happening outside of
test_expect invocations, but they are all innoccuous system
commands like "cat" and "cp". In an ideal world, each test
would be self sufficient and all commands would happen
inside test_expect, but it is not immediately obvious how
the grouping should work (some of the commands impact the
subsequent tests, and some of them are setting up and
modifying state that many tests depend on). This patch just
picks the low-hanging style fruit, and we can do more fixes
on top later.

Signed-off-by: Jeff King <peff@peff.net>

diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index e127f35..feb7430 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -55,11 +55,13 @@ test_expect_success 'uppercase section' '
 	test_cmp expect .git/config
 '
 
-test_expect_success 'replace with non-match' \
-	'git config core.penguin kingpin !blue'
+test_expect_success 'replace with non-match' '
+	git config core.penguin kingpin !blue
+'
 
-test_expect_success 'replace with non-match (actually matching)' \
-	'git config core.penguin "very blue" !kingpin'
+test_expect_success 'replace with non-match (actually matching)' '
+	git config core.penguin "very blue" !kingpin
+'
 
 cat > expect << EOF
 [core]
@@ -108,8 +110,9 @@ baz = multiple \
 lines
 EOF
 
-test_expect_success 'unset with cont. lines' \
-	'git config --unset beta.baz'
+test_expect_success 'unset with cont. lines' '
+	git config --unset beta.baz
+'
 
 cat > expect <<\EOF
 [alpha]
@@ -133,8 +136,9 @@ EOF
 
 cp .git/config .git/config2
 
-test_expect_success 'multiple unset' \
-	'git config --unset-all beta.haha'
+test_expect_success 'multiple unset' '
+	git config --unset-all beta.haha
+'
 
 cat > expect << EOF
 [beta] ; silly comment # another comment
@@ -145,7 +149,9 @@ noIndent= sillyValue ; 'nother silly comment
 [nextSection] noNewline = ouch
 EOF
 
-test_expect_success 'multiple unset is correct' 'test_cmp expect .git/config'
+test_expect_success 'multiple unset is correct' '
+	test_cmp expect .git/config
+'
 
 cp .git/config2 .git/config
 
@@ -156,8 +162,9 @@ test_expect_success '--replace-all missing value' '
 
 rm .git/config2
 
-test_expect_success '--replace-all' \
-	'git config --replace-all beta.haha gamma'
+test_expect_success '--replace-all' '
+	git config --replace-all beta.haha gamma
+'
 
 cat > expect << EOF
 [beta] ; silly comment # another comment
@@ -169,7 +176,9 @@ noIndent= sillyValue ; 'nother silly comment
 [nextSection] noNewline = ouch
 EOF
 
-test_expect_success 'all replaced' 'test_cmp expect .git/config'
+test_expect_success 'all replaced' '
+	test_cmp expect .git/config
+'
 
 cat > expect << EOF
 [beta] ; silly comment # another comment
@@ -200,7 +209,11 @@ test_expect_success 'really really mean test' '
 	test_cmp expect .git/config
 '
 
-test_expect_success 'get value' 'test alpha = $(git config beta.haha)'
+test_expect_success 'get value' '
+	echo alpha >expect &&
+	git config beta.haha >actual &&
+	test_cmp expect actual
+'
 
 cat > expect << EOF
 [beta] ; silly comment # another comment
@@ -231,18 +244,23 @@ test_expect_success 'multivar' '
 	test_cmp expect .git/config
 '
 
-test_expect_success 'non-match' \
-	'git config --get nextsection.nonewline !for'
+test_expect_success 'non-match' '
+	git config --get nextsection.nonewline !for
+'
 
-test_expect_success 'non-match value' \
-	'test wow = $(git config --get nextsection.nonewline !for)'
+test_expect_success 'non-match value' '
+	echo wow >expect &&
+	git config --get nextsection.nonewline !for >actual &&
+	test_cmp expect actual
+'
 
 test_expect_success 'ambiguous get' '
 	test_must_fail git config --get nextsection.nonewline
 '
 
-test_expect_success 'get multivar' \
-	'git config --get-all nextsection.nonewline'
+test_expect_success 'get multivar' '
+	git config --get-all nextsection.nonewline
+'
 
 cat > expect << EOF
 [beta] ; silly comment # another comment
@@ -290,8 +308,9 @@ test_expect_success 'invalid key' 'test_must_fail git config inval.2key blabla'
 
 test_expect_success 'correct key' 'git config 123456.a123 987'
 
-test_expect_success 'hierarchical section' \
-	'git config Version.1.2.3eX.Alpha beta'
+test_expect_success 'hierarchical section' '
+	git config Version.1.2.3eX.Alpha beta
+'
 
 cat > expect << EOF
 [beta] ; silly comment # another comment
@@ -307,7 +326,9 @@ noIndent= sillyValue ; 'nother silly comment
 	Alpha = beta
 EOF
 
-test_expect_success 'hierarchical section value' 'test_cmp expect .git/config'
+test_expect_success 'hierarchical section value' '
+	test_cmp expect .git/config
+'
 
 cat > expect << EOF
 beta.noindent=sillyValue
@@ -316,9 +337,10 @@ nextsection.nonewline=wow2 for me
 version.1.2.3eX.alpha=beta
 EOF
 
-test_expect_success 'working --list' \
-	'git config --list > output && cmp output expect'
-
+test_expect_success 'working --list' '
+	git config --list > output &&
+	test_cmp expect output
+'
 cat > expect << EOF
 EOF
 
@@ -332,8 +354,10 @@ beta.noindent sillyValue
 nextsection.nonewline wow2 for me
 EOF
 
-test_expect_success '--get-regexp' \
-	'git config --get-regexp in > output && cmp output expect'
+test_expect_success '--get-regexp' '
+	git config --get-regexp in >output &&
+	test_cmp expect output
+'
 
 cat > expect << EOF
 wow2 for me
@@ -353,41 +377,48 @@ cat > .git/config << EOF
 	variable =
 EOF
 
-test_expect_success 'get variable with no value' \
-	'git config --get novalue.variable ^$'
+test_expect_success 'get variable with no value' '
+	git config --get novalue.variable ^$
+'
 
-test_expect_success 'get variable with empty value' \
-	'git config --get emptyvalue.variable ^$'
+test_expect_success 'get variable with empty value' '
+	git config --get emptyvalue.variable ^$
+'
 
 echo novalue.variable > expect
 
-test_expect_success 'get-regexp variable with no value' \
-	'git config --get-regexp novalue > output &&
-	 cmp output expect'
+test_expect_success 'get-regexp variable with no value' '
+	git config --get-regexp novalue > output &&
+	test_cmp expect output
+'
 
 echo 'novalue.variable true' > expect
 
-test_expect_success 'get-regexp --bool variable with no value' \
-	'git config --bool --get-regexp novalue > output &&
-	 cmp output expect'
+test_expect_success 'get-regexp --bool variable with no value' '
+	git config --bool --get-regexp novalue > output &&
+	test_cmp expect output
+'
 
 echo 'emptyvalue.variable ' > expect
 
-test_expect_success 'get-regexp variable with empty value' \
-	'git config --get-regexp emptyvalue > output &&
-	 cmp output expect'
+test_expect_success 'get-regexp variable with empty value' '
+	git config --get-regexp emptyvalue > output &&
+	test_cmp expect output
+'
 
 echo true > expect
 
-test_expect_success 'get bool variable with no value' \
-	'git config --bool novalue.variable > output &&
-	 cmp output expect'
+test_expect_success 'get bool variable with no value' '
+	git config --bool novalue.variable > output &&
+	test_cmp expect output
+'
 
 echo false > expect
 
-test_expect_success 'get bool variable with empty value' \
-	'git config --bool emptyvalue.variable > output &&
-	 cmp output expect'
+test_expect_success 'get bool variable with empty value' '
+	git config --bool emptyvalue.variable > output &&
+	test_cmp expect output
+'
 
 test_expect_success 'no arguments, but no crash' '
 	test_must_fail git config >output 2>&1 &&
@@ -427,8 +458,9 @@ test_expect_success 'new variable inserts into proper section' '
 	test_cmp expect .git/config
 '
 
-test_expect_success 'alternative GIT_CONFIG (non-existing file should fail)' \
-	'test_must_fail git config --file non-existing-config -l'
+test_expect_success 'alternative GIT_CONFIG (non-existing file should fail)' '
+	test_must_fail git config --file non-existing-config -l
+'
 
 cat > other-config << EOF
 [ein]
@@ -444,8 +476,10 @@ test_expect_success 'alternative GIT_CONFIG' '
 	test_cmp expect output
 '
 
-test_expect_success 'alternative GIT_CONFIG (--file)' \
-	'git config --file other-config -l > output && cmp output expect'
+test_expect_success 'alternative GIT_CONFIG (--file)' '
+	git config --file other-config -l > output &&
+	test_cmp expect output
+'
 
 test_expect_success 'refer config from subdirectory' '
 	mkdir x &&
@@ -489,8 +523,9 @@ cat > .git/config << EOF
 weird
 EOF
 
-test_expect_success "rename section" \
-	"git config --rename-section branch.eins branch.zwei"
+test_expect_success 'rename section' '
+	git config --rename-section branch.eins branch.zwei
+'
 
 cat > expect << EOF
 # Hallo
@@ -503,17 +538,22 @@ cat > expect << EOF
 weird
 EOF
 
-test_expect_success "rename succeeded" "test_cmp expect .git/config"
+test_expect_success 'rename succeeded' '
+	test_cmp expect .git/config
+'
 
-test_expect_success "rename non-existing section" '
+test_expect_success 'rename non-existing section' '
 	test_must_fail git config --rename-section \
 		branch."world domination" branch.drei
 '
 
-test_expect_success "rename succeeded" "test_cmp expect .git/config"
+test_expect_success 'rename succeeded' '
+	test_cmp expect .git/config
+'
 
-test_expect_success "rename another section" \
-	'git config --rename-section branch."1 234 blabl/a" branch.drei'
+test_expect_success 'rename another section' '
+	git config --rename-section branch."1 234 blabl/a" branch.drei
+'
 
 cat > expect << EOF
 # Hallo
@@ -526,14 +566,17 @@ cat > expect << EOF
 weird
 EOF
 
-test_expect_success "rename succeeded" "test_cmp expect .git/config"
+test_expect_success 'rename succeeded' '
+	test_cmp expect .git/config
+'
 
 cat >> .git/config << EOF
 [branch "vier"] z = 1
 EOF
 
-test_expect_success "rename a section with a var on the same line" \
-	'git config --rename-section branch.vier branch.zwei'
+test_expect_success 'rename a section with a var on the same line' '
+	git config --rename-section branch.vier branch.zwei
+'
 
 cat > expect << EOF
 # Hallo
@@ -548,7 +591,9 @@ weird
 	z = 1
 EOF
 
-test_expect_success "rename succeeded" "test_cmp expect .git/config"
+test_expect_success 'rename succeeded' '
+	test_cmp expect .git/config
+'
 
 test_expect_success 'renaming empty section name is rejected' '
 	test_must_fail git config --rename-section branch.zwei ""
@@ -562,7 +607,9 @@ cat >> .git/config << EOF
   [branch "zwei"] a = 1 [branch "vier"]
 EOF
 
-test_expect_success "remove section" "git config --remove-section branch.zwei"
+test_expect_success 'remove section' '
+	git config --remove-section branch.zwei
+'
 
 cat > expect << EOF
 # Hallo
@@ -571,8 +618,9 @@ cat > expect << EOF
 weird
 EOF
 
-test_expect_success "section was removed properly" \
-	"test_cmp expect .git/config"
+test_expect_success 'section was removed properly' '
+	test_cmp expect .git/config
+'
 
 cat > expect << EOF
 [gitcvs]
@@ -583,7 +631,6 @@ cat > expect << EOF
 EOF
 
 test_expect_success 'section ending' '
-
 	rm -f .git/config &&
 	git config gitcvs.enabled true &&
 	git config gitcvs.ext.dbname %Ggitcvs1.%a.%m.sqlite &&
@@ -593,30 +640,25 @@ test_expect_success 'section ending' '
 '
 
 test_expect_success numbers '
-
 	git config kilo.gram 1k &&
 	git config mega.ton 1m &&
-	k=$(git config --int --get kilo.gram) &&
-	test z1024 = "z$k" &&
-	m=$(git config --int --get mega.ton) &&
-	test z1048576 = "z$m"
+	echo 1024 >expect &&
+	echo 1048576 >>expect &&
+	git config --int --get kilo.gram >actual &&
+	git config --int --get mega.ton >>actual &&
+	test_cmp expect actual
 '
 
-cat > expect <<EOF
-fatal: bad config value for 'aninvalid.unit' in .git/config
-EOF
-
 test_expect_success 'invalid unit' '
-
 	git config aninvalid.unit "1auto" &&
-	s=$(git config aninvalid.unit) &&
-	test "z1auto" = "z$s" &&
-	if git config --int --get aninvalid.unit 2>actual
-	then
-		echo config should have failed
-		false
-	fi &&
-	cmp actual expect
+	echo 1auto >expect &&
+	git config aninvalid.unit >actual &&
+	test_cmp expect actual &&
+	cat > expect <<-\EOF
+	fatal: bad config value for '\''aninvalid.unit'\'' in .git/config
+	EOF
+	test_must_fail git config --int --get aninvalid.unit 2>actual &&
+	test_cmp actual expect
 '
 
 cat > expect << EOF
@@ -646,7 +688,7 @@ test_expect_success bool '
 	    git config --bool --get bool.true$i >>result
 	    git config --bool --get bool.false$i >>result
         done &&
-	cmp expect result'
+	test_cmp expect result'
 
 test_expect_success 'invalid bool (--get)' '
 
@@ -680,7 +722,7 @@ test_expect_success 'set --bool' '
 	git config --bool bool.false2 "" &&
 	git config --bool bool.false3 nO &&
 	git config --bool bool.false4 FALSE &&
-	cmp expect .git/config'
+	test_cmp expect .git/config'
 
 cat > expect <<\EOF
 [int]
@@ -695,39 +737,37 @@ test_expect_success 'set --int' '
 	git config --int int.val1 01 &&
 	git config --int int.val2 -1 &&
 	git config --int int.val3 5m &&
-	cmp expect .git/config'
+	test_cmp expect .git/config
+'
 
-cat >expect <<\EOF
-[bool]
-	true1 = true
+test_expect_success 'get --bool-or-int' '
+	cat >.git/config <<-\EOF &&
+	[bool]
+	true1
 	true2 = true
-	false1 = false
-	false2 = false
-[int]
+	false = false
+	[int]
 	int1 = 0
 	int2 = 1
 	int3 = -1
-EOF
-
-test_expect_success 'get --bool-or-int' '
-	rm -f .git/config &&
-	(
-		echo "[bool]"
-		echo true1
-		echo true2 = true
-		echo false = false
-		echo "[int]"
-		echo int1 = 0
-		echo int2 = 1
-		echo int3 = -1
-	) >>.git/config &&
-	test $(git config --bool-or-int bool.true1) = true &&
-	test $(git config --bool-or-int bool.true2) = true &&
-	test $(git config --bool-or-int bool.false) = false &&
-	test $(git config --bool-or-int int.int1) = 0 &&
-	test $(git config --bool-or-int int.int2) = 1 &&
-	test $(git config --bool-or-int int.int3) = -1
-
+	EOF
+	cat >expect <<-\EOF &&
+	true
+	true
+	false
+	0
+	1
+	-1
+	EOF
+	{
+		git config --bool-or-int bool.true1 &&
+		git config --bool-or-int bool.true2 &&
+		git config --bool-or-int bool.false &&
+		git config --bool-or-int int.int1 &&
+		git config --bool-or-int int.int2 &&
+		git config --bool-or-int int.int3
+	} >actual &&
+	test_cmp expect actual
 '
 
 cat >expect <<\EOF
@@ -844,7 +884,7 @@ EOF
 
 test_expect_success 'value continued on next line' '
 	git config --list > result &&
-	cmp result expect
+	test_cmp result expect
 '
 
 cat > .git/config <<\EOF
@@ -880,11 +920,12 @@ test_expect_success '--null --get-regexp' '
 
 test_expect_success 'inner whitespace kept verbatim' '
 	git config section.val "foo 	  bar" &&
-	test "z$(git config section.val)" = "zfoo 	  bar"
+	echo "foo 	  bar" >expect &&
+	git config section.val >actual &&
+	test_cmp expect actual
 '
 
 test_expect_success SYMLINKS 'symlinked configuration' '
-
 	ln -s notyet myconfig &&
 	GIT_CONFIG=myconfig git config test.frotz nitfol &&
 	test -h myconfig &&
@@ -893,9 +934,15 @@ test_expect_success SYMLINKS 'symlinked configuration' '
 	GIT_CONFIG=myconfig git config test.xyzzy rezrov &&
 	test -h myconfig &&
 	test -f notyet &&
-	test "z$(GIT_CONFIG=notyet git config test.frotz)" = znitfol &&
-	test "z$(GIT_CONFIG=notyet git config test.xyzzy)" = zrezrov
-
+	cat >expect <<-\EOF &&
+	nitfol
+	rezrov
+	EOF
+	{
+		GIT_CONFIG=notyet git config test.frotz &&
+		GIT_CONFIG=notyet git config test.xyzzy
+	} >actual &&
+	test_cmp expect actual
 '
 
 test_expect_success 'nonexistent configuration' '
@@ -927,12 +974,20 @@ test_expect_success 'check split_cmdline return' "
 	git commit -m 'initial commit' &&
 	git config branch.master.mergeoptions 'echo \"' &&
 	test_must_fail git merge master
-	"
+"
 
 test_expect_success 'git -c "key=value" support' '
-	test "z$(git -c core.name=value config core.name)" = zvalue &&
-	test "z$(git -c foo.CamelCase=value config foo.camelcase)" = zvalue &&
-	test "z$(git -c foo.flag config --bool foo.flag)" = ztrue &&
+	cat >expect <<-\EOF &&
+	value
+	value
+	true
+	EOF
+	{
+		git -c core.name=value config core.name &&
+		git -c foo.CamelCase=value config foo.camelcase &&
+		git -c foo.flag config --bool foo.flag
+	} >actual &&
+	test_cmp expect actual &&
 	test_must_fail git -c name=value config core.name
 '
 
-- 
cgit v0.10.2-6-g49f6


From 65ff5301342614cb72639aa8e676e62b77d1165c Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Tue, 23 Oct 2012 15:09:22 -0400
Subject: t1300: remove redundant test

This test checks that git-config fails for an ambiguous
"get", but we check the exact same thing 3 tests beforehand.

Signed-off-by: Jeff King <peff@peff.net>

diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index feb7430..ed75c5c 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -277,10 +277,6 @@ test_expect_success 'multivar replace' '
 	test_cmp expect .git/config
 '
 
-test_expect_success 'ambiguous value' '
-	test_must_fail git config nextsection.nonewline
-'
-
 test_expect_success 'ambiguous unset' '
 	test_must_fail git config --unset nextsection.nonewline
 '
-- 
cgit v0.10.2-6-g49f6


From cb20b69166786210bcad406c192763f90be1639a Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Tue, 23 Oct 2012 18:05:49 -0400
Subject: t1300: test "git config --get-all" more thoroughly

We check that we can "--get-all" a multi-valued variable,
but we do not actually confirm that the output is sensible.
Doing so reveals that it works fine, but this will help us
ensure we do not have regressions in the next few patches,
which will touch this area.

Signed-off-by: Jeff King <peff@peff.net>

diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index ed75c5c..51dd5b5 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -258,8 +258,13 @@ test_expect_success 'ambiguous get' '
 	test_must_fail git config --get nextsection.nonewline
 '
 
-test_expect_success 'get multivar' '
-	git config --get-all nextsection.nonewline
+test_expect_success 'multi-valued get-all returns all' '
+	cat >expect <<-\EOF &&
+	wow
+	wow2 for me
+	EOF
+	git config --get-all nextsection.nonewline >actual &&
+	test_cmp expect actual
 '
 
 cat > expect << EOF
-- 
cgit v0.10.2-6-g49f6


From 35998c89381a56b28433852386986aafde92428d Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Tue, 23 Oct 2012 15:36:12 -0400
Subject: git-config: remove memory leak of key regexp

This is only called once per invocation, so it's not a major
leak, but it's easy to fix.

Signed-off-by: Jeff King <peff@peff.net>

diff --git a/builtin/config.c b/builtin/config.c
index e1c33e0..e660d48 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -248,6 +248,10 @@ static int get_value(const char *key_, const char *regex_)
 		git_config_from_file(fn, system_wide, data);
 
 	free(key);
+	if (key_regexp) {
+		regfree(key_regexp);
+		free(key_regexp);
+	}
 	if (regexp) {
 		regfree(regexp);
 		free(regexp);
-- 
cgit v0.10.2-6-g49f6


From 97ed50f93ba592b50278a8161282e862cb87b4c0 Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Tue, 23 Oct 2012 15:40:06 -0400
Subject: git-config: fix regexp memory leaks on error conditions

The get_value function has a goto label for cleaning up on
errors, but it only cleans up half of what the function
might allocate. Let's also clean up the key and regexp
variables there.

Note that we need to take special care when compiling the
regex fails to clean it up ourselves, since it is in a
half-constructed state (we would want to free it, but not
regfree it).

Similarly, we fix git_config_parse_key to return NULL when
it fails, not a pointer to some already-freed memory.

Signed-off-by: Jeff King <peff@peff.net>

diff --git a/builtin/config.c b/builtin/config.c
index e660d48..60d36e7 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -195,7 +195,8 @@ static int get_value(const char *key_, const char *regex_)
 		key_regexp = (regex_t*)xmalloc(sizeof(regex_t));
 		if (regcomp(key_regexp, key, REG_EXTENDED)) {
 			fprintf(stderr, "Invalid key pattern: %s\n", key_);
-			free(key);
+			free(key_regexp);
+			key_regexp = NULL;
 			ret = CONFIG_INVALID_PATTERN;
 			goto free_strings;
 		}
@@ -215,6 +216,8 @@ static int get_value(const char *key_, const char *regex_)
 		regexp = (regex_t*)xmalloc(sizeof(regex_t));
 		if (regcomp(regexp, regex_, REG_EXTENDED)) {
 			fprintf(stderr, "Invalid pattern: %s\n", regex_);
+			free(regexp);
+			regexp = NULL;
 			ret = CONFIG_INVALID_PATTERN;
 			goto free_strings;
 		}
@@ -247,6 +250,15 @@ static int get_value(const char *key_, const char *regex_)
 	if (!do_all && !seen && system_wide)
 		git_config_from_file(fn, system_wide, data);
 
+	if (do_all)
+		ret = !seen;
+	else
+		ret = (seen == 1) ? 0 : seen > 1 ? 2 : 1;
+
+free_strings:
+	free(repo_config);
+	free(global);
+	free(xdg);
 	free(key);
 	if (key_regexp) {
 		regfree(key_regexp);
@@ -257,15 +269,6 @@ static int get_value(const char *key_, const char *regex_)
 		free(regexp);
 	}
 
-	if (do_all)
-		ret = !seen;
-	else
-		ret = (seen == 1) ? 0 : seen > 1 ? 2 : 1;
-
-free_strings:
-	free(repo_config);
-	free(global);
-	free(xdg);
 	return ret;
 }
 
diff --git a/config.c b/config.c
index 08e47e2..2fbe634 100644
--- a/config.c
+++ b/config.c
@@ -1280,6 +1280,7 @@ int git_config_parse_key(const char *key, char **store_key, int *baselen_)
 
 out_free_ret_1:
 	free(*store_key);
+	*store_key = NULL;
 	return -CONFIG_INVALID_KEY;
 }
 
-- 
cgit v0.10.2-6-g49f6


From 7acdd6f0bc27bba37591e2d5fc2ca84e2af13ce4 Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Tue, 23 Oct 2012 15:51:50 -0400
Subject: git-config: collect values instead of immediately printing

This is a refactor that will allow us to more easily tweak
the behavior for multi-valued variables, and it will
ultimately allow us to remove a lot git-config's custom code
in favor of the regular git_config code.

It does mean we're no longer streaming, and we're storing
more in memory for the --get-all case, but in practice it is
a tiny amount of data, and the results are instantaneous.

Signed-off-by: Jeff King <peff@peff.net>

diff --git a/builtin/config.c b/builtin/config.c
index 60d36e7..08e83fc 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -15,7 +15,6 @@ static int show_keys;
 static int use_key_regexp;
 static int do_all;
 static int do_not_match;
-static int seen;
 static char delim = '=';
 static char key_delim = ' ';
 static char term = '\n';
@@ -95,8 +94,16 @@ static int show_all_config(const char *key_, const char *value_, void *cb)
 	return 0;
 }
 
-static int show_config(const char *key_, const char *value_, void *cb)
+struct strbuf_list {
+	struct strbuf *items;
+	int nr;
+	int alloc;
+};
+
+static int collect_config(const char *key_, const char *value_, void *cb)
 {
+	struct strbuf_list *values = cb;
+	struct strbuf *buf;
 	char value[256];
 	const char *vptr = value;
 	int must_free_vptr = 0;
@@ -111,11 +118,15 @@ static int show_config(const char *key_, const char *value_, void *cb)
 	    (do_not_match ^ !!regexec(regexp, (value_?value_:""), 0, NULL, 0)))
 		return 0;
 
+	ALLOC_GROW(values->items, values->nr + 1, values->alloc);
+	buf = &values->items[values->nr++];
+	strbuf_init(buf, 0);
+
 	if (show_keys) {
-		printf("%s", key_);
+		strbuf_addstr(buf, key_);
 		must_print_delim = 1;
 	}
-	if (seen && !do_all)
+	if (values->nr > 1 && !do_all)
 		dup_error = 1;
 	if (types == TYPE_INT)
 		sprintf(value, "%d", git_config_int(key_, value_?value_:""));
@@ -138,15 +149,15 @@ static int show_config(const char *key_, const char *value_, void *cb)
 		vptr = "";
 		must_print_delim = 0;
 	}
-	seen++;
 	if (dup_error) {
 		error("More than one value for the key %s: %s",
 				key_, vptr);
 	}
 	else {
 		if (must_print_delim)
-			printf("%c", key_delim);
-		printf("%s%c", vptr, term);
+			strbuf_addch(buf, key_delim);
+		strbuf_addstr(buf, vptr);
+		strbuf_addch(buf, term);
 	}
 	if (must_free_vptr)
 		/* If vptr must be freed, it's a pointer to a
@@ -166,6 +177,8 @@ static int get_value(const char *key_, const char *regex_)
 	struct config_include_data inc = CONFIG_INCLUDE_INIT;
 	config_fn_t fn;
 	void *data;
+	struct strbuf_list values = {0};
+	int i;
 
 	local = given_config_file;
 	if (!local) {
@@ -223,8 +236,8 @@ static int get_value(const char *key_, const char *regex_)
 		}
 	}
 
-	fn = show_config;
-	data = NULL;
+	fn = collect_config;
+	data = &values;
 	if (respect_includes) {
 		inc.fn = fn;
 		inc.data = data;
@@ -241,19 +254,26 @@ static int get_value(const char *key_, const char *regex_)
 	if (do_all)
 		git_config_from_file(fn, local, data);
 	git_config_from_parameters(fn, data);
-	if (!do_all && !seen)
+	if (!do_all && !values.nr)
 		git_config_from_file(fn, local, data);
-	if (!do_all && !seen && global)
+	if (!do_all && !values.nr && global)
 		git_config_from_file(fn, global, data);
-	if (!do_all && !seen && xdg)
+	if (!do_all && !values.nr && xdg)
 		git_config_from_file(fn, xdg, data);
-	if (!do_all && !seen && system_wide)
+	if (!do_all && !values.nr && system_wide)
 		git_config_from_file(fn, system_wide, data);
 
 	if (do_all)
-		ret = !seen;
+		ret = !values.nr;
 	else
-		ret = (seen == 1) ? 0 : seen > 1 ? 2 : 1;
+		ret = (values.nr == 1) ? 0 : values.nr > 1 ? 2 : 1;
+
+	for (i = 0; i < values.nr; i++) {
+		struct strbuf *buf = values.items + i;
+		fwrite(buf->buf, 1, buf->len, stdout);
+		strbuf_release(buf);
+	}
+	free(values.items);
 
 free_strings:
 	free(repo_config);
-- 
cgit v0.10.2-6-g49f6


From 00b347d3aa094948d58dd1303aef7ba97ff09519 Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Tue, 23 Oct 2012 16:52:44 -0400
Subject: git-config: do not complain about duplicate entries

If git-config is asked for a single value, it will complain
and exit with an error if it finds multiple instances of
that value. This is unlike the usual internal config
parsing, however, which will generally overwrite previous
values, leaving only the final one. For example:

  [set a multivar]
  $ git config user.email one@example.com
  $ git config --add user.email two@example.com

  [use the internal parser to fetch it]
  $ git var GIT_AUTHOR_IDENT
  Your Name <two@example.com> ...

  [use git-config to fetch it]
  $ git config user.email
  one@example.com
  error: More than one value for the key user.email: two@example.com

This overwriting behavior is critical for the regular
parser, which starts with the lowest-priority file (e.g.,
/etc/gitconfig) and proceeds to the highest-priority file
($GIT_DIR/config). Overwriting yields the highest priority
value at the end.

Git-config solves this problem by implementing its own
parsing. It goes from highest to lowest priorty, but does
not proceed to the next file if it has seen a value.

So in practice, this distinction never mattered much,
because it only triggered for values in the same file. And
there was not much point in doing that; the real value is in
overwriting values from lower-priority files.

However, this changed with the implementation of config
include files. Now we might see an include overriding a
value from the parent file, which is a sensible thing to do,
but git-config will flag as a duplication.

This patch drops the duplicate detection for git-config and
switches to a pure-overwrite model (for the single case;
--get-all can still be used if callers want to do something
more fancy).

As is shown by the modifications to the test suite, this is
a user-visible change in behavior. An alternative would be
to just change the include case, but this is much cleaner
for a few reasons:

  1. If you change the include case, then to what? If you
     just stop parsing includes after getting a value, then
     you will get a _different_ answer than the regular
     config parser (you'll get the first value instead of
     the last value). So you'd want to implement overwrite
     semantics anyway.

  2. Even though it is a change in behavior for git-config,
     it is bringing us in line with what the internal
     parsers already do.

  3. The file-order reimplementation is the only thing
     keeping us from sharing more code with the internal
     config parser, which will help keep differences to a
     minimum.

Going under the assumption that the primary purpose of
git-config is to behave identically to how git's internal
parsing works, this change can be seen as a bug-fix.

Signed-off-by: Jeff King <peff@peff.net>

diff --git a/builtin/config.c b/builtin/config.c
index 08e83fc..77efa69 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -107,7 +107,6 @@ static int collect_config(const char *key_, const char *value_, void *cb)
 	char value[256];
 	const char *vptr = value;
 	int must_free_vptr = 0;
-	int dup_error = 0;
 	int must_print_delim = 0;
 
 	if (!use_key_regexp && strcmp(key_, key))
@@ -126,8 +125,6 @@ static int collect_config(const char *key_, const char *value_, void *cb)
 		strbuf_addstr(buf, key_);
 		must_print_delim = 1;
 	}
-	if (values->nr > 1 && !do_all)
-		dup_error = 1;
 	if (types == TYPE_INT)
 		sprintf(value, "%d", git_config_int(key_, value_?value_:""));
 	else if (types == TYPE_BOOL)
@@ -149,16 +146,12 @@ static int collect_config(const char *key_, const char *value_, void *cb)
 		vptr = "";
 		must_print_delim = 0;
 	}
-	if (dup_error) {
-		error("More than one value for the key %s: %s",
-				key_, vptr);
-	}
-	else {
-		if (must_print_delim)
-			strbuf_addch(buf, key_delim);
-		strbuf_addstr(buf, vptr);
-		strbuf_addch(buf, term);
-	}
+
+	if (must_print_delim)
+		strbuf_addch(buf, key_delim);
+	strbuf_addstr(buf, vptr);
+	strbuf_addch(buf, term);
+
 	if (must_free_vptr)
 		/* If vptr must be freed, it's a pointer to a
 		 * dynamically allocated buffer, it's safe to cast to
@@ -263,14 +256,12 @@ static int get_value(const char *key_, const char *regex_)
 	if (!do_all && !values.nr && system_wide)
 		git_config_from_file(fn, system_wide, data);
 
-	if (do_all)
-		ret = !values.nr;
-	else
-		ret = (values.nr == 1) ? 0 : values.nr > 1 ? 2 : 1;
+	ret = !values.nr;
 
 	for (i = 0; i < values.nr; i++) {
 		struct strbuf *buf = values.items + i;
-		fwrite(buf->buf, 1, buf->len, stdout);
+		if (do_all || i == values.nr - 1)
+			fwrite(buf->buf, 1, buf->len, stdout);
 		strbuf_release(buf);
 	}
 	free(values.items);
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 51dd5b5..97a25f1 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -254,8 +254,10 @@ test_expect_success 'non-match value' '
 	test_cmp expect actual
 '
 
-test_expect_success 'ambiguous get' '
-	test_must_fail git config --get nextsection.nonewline
+test_expect_success 'multi-valued get returns final one' '
+	echo "wow2 for me" >expect &&
+	git config --get nextsection.nonewline >actual &&
+	test_cmp expect actual
 '
 
 test_expect_success 'multi-valued get-all returns all' '
diff --git a/t/t9700/test.pl b/t/t9700/test.pl
index 3b9b484..0d4e366 100755
--- a/t/t9700/test.pl
+++ b/t/t9700/test.pl
@@ -46,8 +46,7 @@ is($r->get_color("color.test.slot1", "red"), $ansi_green, "get_color");
 # Save and restore STDERR; we will probably extract this into a
 # "dies_ok" method and possibly move the STDERR handling to Git.pm.
 open our $tmpstderr, ">&STDERR" or die "cannot save STDERR"; close STDERR;
-eval { $r->config("test.dupstring") };
-ok($@, "config: duplicate entry in scalar context fails");
+is($r->config("test.dupstring"), "value2", "config: multivar");
 eval { $r->config_bool("test.boolother") };
 ok($@, "config_bool: non-boolean values fail");
 open STDERR, ">&", $tmpstderr or die "cannot restore STDERR";
-- 
cgit v0.10.2-6-g49f6


From e89558988305c3e912c0674faeb814e44563f7fd Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Tue, 23 Oct 2012 18:00:41 -0400
Subject: git-config: use git_config_with_options

The git-config command has always implemented its own file
lookup and parsing order. This was necessary because its
duplicate-entry handling did not match the way git's
internal callbacks worked. Now that this is no longer the
case, we are free to reuse the existing parsing code.

This saves us a few lines of code, but most importantly, it
means that the logic for which files are examined is
contained only in one place and cannot diverge.

Signed-off-by: Jeff King <peff@peff.net>

diff --git a/builtin/config.c b/builtin/config.c
index 77efa69..f881053 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -165,22 +165,9 @@ static int collect_config(const char *key_, const char *value_, void *cb)
 static int get_value(const char *key_, const char *regex_)
 {
 	int ret = CONFIG_GENERIC_ERROR;
-	char *global = NULL, *xdg = NULL, *repo_config = NULL;
-	const char *system_wide = NULL, *local;
-	struct config_include_data inc = CONFIG_INCLUDE_INIT;
-	config_fn_t fn;
-	void *data;
 	struct strbuf_list values = {0};
 	int i;
 
-	local = given_config_file;
-	if (!local) {
-		local = repo_config = git_pathdup("config");
-		if (git_config_system())
-			system_wide = git_etc_gitconfig();
-		home_config_paths(&global, &xdg, "config");
-	}
-
 	if (use_key_regexp) {
 		char *tl;
 
@@ -229,32 +216,8 @@ static int get_value(const char *key_, const char *regex_)
 		}
 	}
 
-	fn = collect_config;
-	data = &values;
-	if (respect_includes) {
-		inc.fn = fn;
-		inc.data = data;
-		fn = git_config_include;
-		data = &inc;
-	}
-
-	if (do_all && system_wide)
-		git_config_from_file(fn, system_wide, data);
-	if (do_all && xdg)
-		git_config_from_file(fn, xdg, data);
-	if (do_all && global)
-		git_config_from_file(fn, global, data);
-	if (do_all)
-		git_config_from_file(fn, local, data);
-	git_config_from_parameters(fn, data);
-	if (!do_all && !values.nr)
-		git_config_from_file(fn, local, data);
-	if (!do_all && !values.nr && global)
-		git_config_from_file(fn, global, data);
-	if (!do_all && !values.nr && xdg)
-		git_config_from_file(fn, xdg, data);
-	if (!do_all && !values.nr && system_wide)
-		git_config_from_file(fn, system_wide, data);
+	git_config_with_options(collect_config, &values,
+				given_config_file, respect_includes);
 
 	ret = !values.nr;
 
@@ -267,9 +230,6 @@ static int get_value(const char *key_, const char *regex_)
 	free(values.items);
 
 free_strings:
-	free(repo_config);
-	free(global);
-	free(xdg);
 	free(key);
 	if (key_regexp) {
 		regfree(key_regexp);
-- 
cgit v0.10.2-6-g49f6


From 5ba1a8a735c721cc66cc030866163f6fdb7f5ecb Mon Sep 17 00:00:00 2001
From: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Date: Sun, 28 Oct 2012 21:05:25 +0000
Subject: builtin/config.c: Fix a sparse warning

Sparse issues an "Using plain integer as NULL pointer" warning while
checking a 'struct strbuf_list' initializer expression. The initial
field of the struct has pointer type, but the initializer expression
is given as '{0}'. In order to suppress the warning, we simply replace
the initializer with '{NULL}'.

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Signed-off-by: Jeff King <peff@peff.net>

diff --git a/builtin/config.c b/builtin/config.c
index f881053..e796af4 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -165,7 +165,7 @@ static int collect_config(const char *key_, const char *value_, void *cb)
 static int get_value(const char *key_, const char *regex_)
 {
 	int ret = CONFIG_GENERIC_ERROR;
-	struct strbuf_list values = {0};
+	struct strbuf_list values = {NULL};
 	int i;
 
 	if (use_key_regexp) {
-- 
cgit v0.10.2-6-g49f6