git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Bug with git svn fetch? "error:too many matches for svn-remote.svn.added-placeholder"
@ 2013-11-05 15:31 Jess Hottenstein
  2013-11-13 10:19 ` [PATCH] config: arbitrary number of matches for --unset and --replace-all Thomas Rast
  0 siblings, 1 reply; 8+ messages in thread
From: Jess Hottenstein @ 2013-11-05 15:31 UTC (permalink / raw)
  To: git

Hello,

I'm running a git svn fetch of a large svn repo and it is grinding to
a halt with thousands of "error: too many matches for
svn-remote.svn.added-placeholder".  My .git/config has grown to ~50000
lines of added-placeholders.

I also had to up my linux file limit because I have ~2500 open files
(tempfiles in .git)

I'm doing a onetime migration so I don't really care about having the
added-placeholder entries but I do want to keep empty directories.

Any thoughts for a workaround?

Thanks!

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH] config: arbitrary number of matches for --unset and --replace-all
  2013-11-05 15:31 Bug with git svn fetch? "error:too many matches for svn-remote.svn.added-placeholder" Jess Hottenstein
@ 2013-11-13 10:19 ` Thomas Rast
  2013-11-13 23:22   ` Eric Sunshine
  2013-12-06 18:46   ` [PATCH] fixup! " Junio C Hamano
  0 siblings, 2 replies; 8+ messages in thread
From: Thomas Rast @ 2013-11-13 10:19 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano

git-config used a static match array to hold the matches we want to
unset/replace when using --unset or --replace-all.  Use a
variable-sized array instead.

This in particular fixes the symptoms git-svn had when storing large
numbers of svn-remote.*.added-placeholder entries in the config file.

While the tests are rather more paranoid than just --unset and
--replace-all, the other operations already worked.  Indeed git-svn's
usage only breaks the first time *after* creating so many entries,
when it wants to unset and re-add them all.

Reported-by: Jess Hottenstein <jess.hottenstein@gmail.com>
Signed-off-by: Thomas Rast <tr@thomasrast.ch>
---

This should fix it, though I haven't actually tested with such a funny
use-case, nor written a proper git-svn test for it.  My analysis about
the failure mode is from briefly looking at the code.

git-svn should probably be changed to use another way of storing
these, as git-config is not a very efficient database, but I'm leaving
that for someone who cares about SVN.  (Note that it's also much
harder as it'll need a migration plan.)


 config.c                | 19 ++++++++++-----
 t/t1303-wacky-config.sh | 64 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 77 insertions(+), 6 deletions(-)

diff --git a/config.c b/config.c
index 3c2434a..ef63bf2 100644
--- a/config.c
+++ b/config.c
@@ -1197,15 +1197,14 @@ int git_config(config_fn_t fn, void *data)
  * Find all the stuff for git_config_set() below.
  */
 
-#define MAX_MATCHES 512
-
 static struct {
 	int baselen;
 	char *key;
 	int do_not_match;
 	regex_t *value_regex;
 	int multi_replace;
-	size_t offset[MAX_MATCHES];
+	size_t *offset;
+	unsigned int offset_alloc;
 	enum { START, SECTION_SEEN, SECTION_END_SEEN, KEY_SEEN } state;
 	int seen;
 } store;
@@ -1228,11 +1227,11 @@ static int store_aux(const char *key, const char *value, void *cb)
 		if (matches(key, value)) {
 			if (store.seen == 1 && store.multi_replace == 0) {
 				warning("%s has multiple values", key);
-			} else if (store.seen >= MAX_MATCHES) {
-				error("too many matches for %s", key);
-				return 1;
 			}
 
+			ALLOC_GROW(store.offset, store.seen+1,
+				   store.offset_alloc);
+
 			store.offset[store.seen] = cf->do_ftell(cf);
 			store.seen++;
 		}
@@ -1260,11 +1259,15 @@ static int store_aux(const char *key, const char *value, void *cb)
 		 * Do not increment matches: this is no match, but we
 		 * just made sure we are in the desired section.
 		 */
+		ALLOC_GROW(store.offset, store.seen+1,
+			   store.offset_alloc);
 		store.offset[store.seen] = cf->do_ftell(cf);
 		/* fallthru */
 	case SECTION_END_SEEN:
 	case START:
 		if (matches(key, value)) {
+			ALLOC_GROW(store.offset, store.seen+1,
+				   store.offset_alloc);
 			store.offset[store.seen] = cf->do_ftell(cf);
 			store.state = KEY_SEEN;
 			store.seen++;
@@ -1272,6 +1275,9 @@ static int store_aux(const char *key, const char *value, void *cb)
 			if (strrchr(key, '.') - key == store.baselen &&
 			      !strncmp(key, store.key, store.baselen)) {
 					store.state = SECTION_SEEN;
+					ALLOC_GROW(store.offset,
+						   store.seen+1,
+						   store.offset_alloc);
 					store.offset[store.seen] = cf->do_ftell(cf);
 			}
 		}
@@ -1570,6 +1576,7 @@ int git_config_set_multivar_in_file(const char *config_filename,
 			}
 		}
 
+		ALLOC_GROW(store.offset, 1, store.offset_alloc);
 		store.offset[0] = 0;
 		store.state = START;
 		store.seen = 0;
diff --git a/t/t1303-wacky-config.sh b/t/t1303-wacky-config.sh
index 46103a1..7d55730 100755
--- a/t/t1303-wacky-config.sh
+++ b/t/t1303-wacky-config.sh
@@ -3,17 +3,28 @@
 test_description='Test wacky input to git config'
 . ./test-lib.sh
 
+# Leaving off the newline is intentional!
 setup() {
 	(printf "[section]\n" &&
 	printf "  key = foo") >.git/config
 }
 
+# 'check section.key value' verifies that the entry for section.key is
+# 'value'
 check() {
 	echo "$2" >expected
 	git config --get "$1" >actual 2>&1
 	test_cmp actual expected
 }
 
+# 'check section.key regex value' verifies that the entry for
+# section.key *that matches 'regex'* is 'value'
+check_regex() {
+	echo "$3" >expected
+	git config --get "$1" "$2" >actual 2>&1
+	test_cmp actual expected
+}
+
 test_expect_success 'modify same key' '
 	setup &&
 	git config section.key bar &&
@@ -47,4 +58,57 @@ test_expect_success 'do not crash on special long config line' '
 	check section.key "$LONG_VALUE"
 '
 
+setup_many() {
+	setup &&
+	# This time we want the newline so that we can tack on more
+	# entries.
+	echo >>.git/config &&
+	# Semi-efficient way of concatenating 5^5 = 3125 lines. Note
+	# that because 'setup' already put one line, this means 3126
+	# entries for section.key in the config file.
+	cat >5to1 <<EOF
+  key = foo
+  key = foo
+  key = foo
+  key = foo
+  key = foo
+EOF
+	cat 5to1 5to1 5to1 5to1 5to1 >5to2 &&	   # 25
+	cat 5to2 5to2 5to2 5to2 5to2 >5to3 &&	   # 125
+	cat 5to3 5to3 5to3 5to3 5to3 >5to4 &&	   # 635
+	cat 5to4 5to4 5to4 5to4 5to4 >>.git/config # 3125
+}
+
+test_expect_success 'get many entries' '
+	setup_many &&
+	git config --get-all section.key >actual &&
+	test_line_count = 3126 actual
+'
+
+test_expect_success 'get many entries by regex' '
+	setup_many &&
+	git config --get-regexp "sec.*ke." >actual &&
+	test_line_count = 3126 actual
+'
+
+test_expect_success 'add and replace one of many entries' '
+	setup_many &&
+	git config --add section.key bar &&
+	check_regex section.key "b.*r" bar &&
+	git config section.key beer "b.*r" &&
+	check_regex section.key "b.*r" beer
+'
+
+test_expect_success 'replace many entries' '
+	setup_many &&
+	git config --replace-all section.key bar &&
+	check section.key bar
+'
+
+test_expect_success 'unset many entries' '
+	setup_many &&
+	git config --unset-all section.key &&
+	test_must_fail git config section.key
+'
+
 test_done
-- 
1.8.5.rc0.346.g150976e

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] config: arbitrary number of matches for --unset and --replace-all
  2013-11-13 10:19 ` [PATCH] config: arbitrary number of matches for --unset and --replace-all Thomas Rast
@ 2013-11-13 23:22   ` Eric Sunshine
  2013-11-14  7:50     ` Thomas Rast
  2013-12-06 18:46   ` [PATCH] fixup! " Junio C Hamano
  1 sibling, 1 reply; 8+ messages in thread
From: Eric Sunshine @ 2013-11-13 23:22 UTC (permalink / raw)
  To: Thomas Rast; +Cc: Git List, Jeff King, Junio C Hamano

On Wed, Nov 13, 2013 at 5:19 AM, Thomas Rast <tr@thomasrast.ch> wrote:
> diff --git a/t/t1303-wacky-config.sh b/t/t1303-wacky-config.sh
> index 46103a1..7d55730 100755
> --- a/t/t1303-wacky-config.sh
> +++ b/t/t1303-wacky-config.sh
> @@ -47,4 +58,57 @@ test_expect_success 'do not crash on special long config line' '
>         check section.key "$LONG_VALUE"
>  '
>
> +setup_many() {
> +       setup &&
> +       # This time we want the newline so that we can tack on more
> +       # entries.
> +       echo >>.git/config &&
> +       # Semi-efficient way of concatenating 5^5 = 3125 lines. Note
> +       # that because 'setup' already put one line, this means 3126
> +       # entries for section.key in the config file.
> +       cat >5to1 <<EOF

Broken &&-chain.

> +  key = foo
> +  key = foo
> +  key = foo
> +  key = foo
> +  key = foo
> +EOF
> +       cat 5to1 5to1 5to1 5to1 5to1 >5to2 &&      # 25
> +       cat 5to2 5to2 5to2 5to2 5to2 >5to3 &&      # 125
> +       cat 5to3 5to3 5to3 5to3 5to3 >5to4 &&      # 635
> +       cat 5to4 5to4 5to4 5to4 5to4 >>.git/config # 3125
> +}
> +
> +test_expect_success 'get many entries' '
> +       setup_many &&
> +       git config --get-all section.key >actual &&
> +       test_line_count = 3126 actual
> +'
> +
> +test_expect_success 'get many entries by regex' '
> +       setup_many &&
> +       git config --get-regexp "sec.*ke." >actual &&
> +       test_line_count = 3126 actual
> +'
> +
> +test_expect_success 'add and replace one of many entries' '
> +       setup_many &&
> +       git config --add section.key bar &&
> +       check_regex section.key "b.*r" bar &&
> +       git config section.key beer "b.*r" &&
> +       check_regex section.key "b.*r" beer
> +'
> +
> +test_expect_success 'replace many entries' '
> +       setup_many &&
> +       git config --replace-all section.key bar &&
> +       check section.key bar
> +'
> +
> +test_expect_success 'unset many entries' '
> +       setup_many &&
> +       git config --unset-all section.key &&
> +       test_must_fail git config section.key
> +'
> +
>  test_done
> --
> 1.8.5.rc0.346.g150976e

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH] config: arbitrary number of matches for --unset and --replace-all
  2013-11-13 23:22   ` Eric Sunshine
@ 2013-11-14  7:50     ` Thomas Rast
  2013-11-14  8:37       ` Jeff King
  0 siblings, 1 reply; 8+ messages in thread
From: Thomas Rast @ 2013-11-14  7:50 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano, Eric Sunshine, Jess Hottenstein

git-config used a static match array to hold the matches we want to
unset/replace when using --unset or --replace-all.  Use a
variable-sized array instead.

This in particular fixes the symptoms git-svn had when storing large
numbers of svn-remote.*.added-placeholder entries in the config file.

While the tests are rather more paranoid than just --unset and
--replace-all, the other operations already worked.  Indeed git-svn's
usage only breaks the first time *after* creating so many entries,
when it wants to unset and re-add them all.

Reported-by: Jess Hottenstein <jess.hottenstein@gmail.com>
Signed-off-by: Thomas Rast <tr@thomasrast.ch>
---

Eric Sunshine wrote:
> On Wed, Nov 13, 2013 at 5:19 AM, Thomas Rast <tr@thomasrast.ch> wrote:
> > +setup_many() {
[...]
> > +       cat >5to1 <<EOF
> 
> Broken &&-chain.

Oops, thanks for catching.


 config.c                | 19 ++++++++++-----
 t/t1303-wacky-config.sh | 64 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 77 insertions(+), 6 deletions(-)

diff --git a/config.c b/config.c
index 3c2434a..ef63bf2 100644
--- a/config.c
+++ b/config.c
@@ -1197,15 +1197,14 @@ int git_config(config_fn_t fn, void *data)
  * Find all the stuff for git_config_set() below.
  */
 
-#define MAX_MATCHES 512
-
 static struct {
 	int baselen;
 	char *key;
 	int do_not_match;
 	regex_t *value_regex;
 	int multi_replace;
-	size_t offset[MAX_MATCHES];
+	size_t *offset;
+	unsigned int offset_alloc;
 	enum { START, SECTION_SEEN, SECTION_END_SEEN, KEY_SEEN } state;
 	int seen;
 } store;
@@ -1228,11 +1227,11 @@ static int store_aux(const char *key, const char *value, void *cb)
 		if (matches(key, value)) {
 			if (store.seen == 1 && store.multi_replace == 0) {
 				warning("%s has multiple values", key);
-			} else if (store.seen >= MAX_MATCHES) {
-				error("too many matches for %s", key);
-				return 1;
 			}
 
+			ALLOC_GROW(store.offset, store.seen+1,
+				   store.offset_alloc);
+
 			store.offset[store.seen] = cf->do_ftell(cf);
 			store.seen++;
 		}
@@ -1260,11 +1259,15 @@ static int store_aux(const char *key, const char *value, void *cb)
 		 * Do not increment matches: this is no match, but we
 		 * just made sure we are in the desired section.
 		 */
+		ALLOC_GROW(store.offset, store.seen+1,
+			   store.offset_alloc);
 		store.offset[store.seen] = cf->do_ftell(cf);
 		/* fallthru */
 	case SECTION_END_SEEN:
 	case START:
 		if (matches(key, value)) {
+			ALLOC_GROW(store.offset, store.seen+1,
+				   store.offset_alloc);
 			store.offset[store.seen] = cf->do_ftell(cf);
 			store.state = KEY_SEEN;
 			store.seen++;
@@ -1272,6 +1275,9 @@ static int store_aux(const char *key, const char *value, void *cb)
 			if (strrchr(key, '.') - key == store.baselen &&
 			      !strncmp(key, store.key, store.baselen)) {
 					store.state = SECTION_SEEN;
+					ALLOC_GROW(store.offset,
+						   store.seen+1,
+						   store.offset_alloc);
 					store.offset[store.seen] = cf->do_ftell(cf);
 			}
 		}
@@ -1570,6 +1576,7 @@ int git_config_set_multivar_in_file(const char *config_filename,
 			}
 		}
 
+		ALLOC_GROW(store.offset, 1, store.offset_alloc);
 		store.offset[0] = 0;
 		store.state = START;
 		store.seen = 0;
diff --git a/t/t1303-wacky-config.sh b/t/t1303-wacky-config.sh
index 46103a1..c2706ea 100755
--- a/t/t1303-wacky-config.sh
+++ b/t/t1303-wacky-config.sh
@@ -3,17 +3,28 @@
 test_description='Test wacky input to git config'
 . ./test-lib.sh
 
+# Leaving off the newline is intentional!
 setup() {
 	(printf "[section]\n" &&
 	printf "  key = foo") >.git/config
 }
 
+# 'check section.key value' verifies that the entry for section.key is
+# 'value'
 check() {
 	echo "$2" >expected
 	git config --get "$1" >actual 2>&1
 	test_cmp actual expected
 }
 
+# 'check section.key regex value' verifies that the entry for
+# section.key *that matches 'regex'* is 'value'
+check_regex() {
+	echo "$3" >expected
+	git config --get "$1" "$2" >actual 2>&1
+	test_cmp actual expected
+}
+
 test_expect_success 'modify same key' '
 	setup &&
 	git config section.key bar &&
@@ -47,4 +58,57 @@ test_expect_success 'do not crash on special long config line' '
 	check section.key "$LONG_VALUE"
 '
 
+setup_many() {
+	setup &&
+	# This time we want the newline so that we can tack on more
+	# entries.
+	echo >>.git/config &&
+	# Semi-efficient way of concatenating 5^5 = 3125 lines. Note
+	# that because 'setup' already put one line, this means 3126
+	# entries for section.key in the config file.
+	cat >5to1 <<EOF &&
+  key = foo
+  key = foo
+  key = foo
+  key = foo
+  key = foo
+EOF
+	cat 5to1 5to1 5to1 5to1 5to1 >5to2 &&	   # 25
+	cat 5to2 5to2 5to2 5to2 5to2 >5to3 &&	   # 125
+	cat 5to3 5to3 5to3 5to3 5to3 >5to4 &&	   # 635
+	cat 5to4 5to4 5to4 5to4 5to4 >>.git/config # 3125
+}
+
+test_expect_success 'get many entries' '
+	setup_many &&
+	git config --get-all section.key >actual &&
+	test_line_count = 3126 actual
+'
+
+test_expect_success 'get many entries by regex' '
+	setup_many &&
+	git config --get-regexp "sec.*ke." >actual &&
+	test_line_count = 3126 actual
+'
+
+test_expect_success 'add and replace one of many entries' '
+	setup_many &&
+	git config --add section.key bar &&
+	check_regex section.key "b.*r" bar &&
+	git config section.key beer "b.*r" &&
+	check_regex section.key "b.*r" beer
+'
+
+test_expect_success 'replace many entries' '
+	setup_many &&
+	git config --replace-all section.key bar &&
+	check section.key bar
+'
+
+test_expect_success 'unset many entries' '
+	setup_many &&
+	git config --unset-all section.key &&
+	test_must_fail git config section.key
+'
+
 test_done
-- 
1.8.5.rc1.353.g06ba152

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] config: arbitrary number of matches for --unset and --replace-all
  2013-11-14  7:50     ` Thomas Rast
@ 2013-11-14  8:37       ` Jeff King
  2013-11-14 20:24         ` Thomas Rast
  0 siblings, 1 reply; 8+ messages in thread
From: Jeff King @ 2013-11-14  8:37 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git, Junio C Hamano, Eric Sunshine, Jess Hottenstein

On Thu, Nov 14, 2013 at 08:50:43AM +0100, Thomas Rast wrote:

> git-config used a static match array to hold the matches we want to
> unset/replace when using --unset or --replace-all.  Use a
> variable-sized array instead.
> 
> This in particular fixes the symptoms git-svn had when storing large
> numbers of svn-remote.*.added-placeholder entries in the config file.
> 
> While the tests are rather more paranoid than just --unset and
> --replace-all, the other operations already worked.  Indeed git-svn's
> usage only breaks the first time *after* creating so many entries,
> when it wants to unset and re-add them all.
> 
> Reported-by: Jess Hottenstein <jess.hottenstein@gmail.com>
> Signed-off-by: Thomas Rast <tr@thomasrast.ch>

This looks good to me.

I agree with your earlier assessment that adding tons of config keys is
probably a bad idea. It's not just slow when looking up those keys, but
it also slows down every single git operation (which might read config
many times, if it is a script).  Still, we should at least do the right
thing in the face of such config.

> @@ -1260,11 +1259,15 @@ static int store_aux(const char *key, const char *value, void *cb)
>  		 * Do not increment matches: this is no match, but we
>  		 * just made sure we are in the desired section.
>  		 */
> +		ALLOC_GROW(store.offset, store.seen+1,
> +			   store.offset_alloc);
>  		store.offset[store.seen] = cf->do_ftell(cf);
>  		/* fallthru */
>  	case SECTION_END_SEEN:
>  	case START:
>  		if (matches(key, value)) {
> +			ALLOC_GROW(store.offset, store.seen+1,
> +				   store.offset_alloc);
>  			store.offset[store.seen] = cf->do_ftell(cf);
>  			store.state = KEY_SEEN;
>  			store.seen++;

This code is weird to follow because of the fall-throughs. I do not
think you have introduced any bugs with your patch, but it seems weird
to me that we set the offset at the top of the hunk. If we hit the
conditional in the bottom half, we do actually increment storer.seen,
but only _after_ having overwritten the value from above (with the same
value, no less).

But if we do not follow that code path, we may end up here:

> @@ -1272,6 +1275,9 @@ static int store_aux(const char *key, const char *value, void *cb)
>  			if (strrchr(key, '.') - key == store.baselen &&
>  			      !strncmp(key, store.key, store.baselen)) {
>  					store.state = SECTION_SEEN;
> +					ALLOC_GROW(store.offset,
> +						   store.seen+1,
> +						   store.offset_alloc);
>  					store.offset[store.seen] = cf->do_ftell(cf);
>  			}
>  		}

where we overwrite it again, but do not update store.seen. Or we may
trigger neither, and leave the function with our offset stored, but
store.seen not incremented.

So it seems odd to me that we would set the offset, but in some cases
never actually increment our counter. AFAICT, we do not ever access
those values. The reader limits itself to "i < store.seen", which makes
sense. And the writers always call a fresh ftell before incrementing
store.seen.  But maybe I am missing something.

Anyway, it is neither here nor there for your patch. Just something odd
I noticed while reading the code.

> +setup_many() {
> +	setup &&
> +	# This time we want the newline so that we can tack on more
> +	# entries.
> +	echo >>.git/config &&
> +	# Semi-efficient way of concatenating 5^5 = 3125 lines. Note
> +	# that because 'setup' already put one line, this means 3126
> +	# entries for section.key in the config file.
> +	cat >5to1 <<EOF &&
> +  key = foo
> +  key = foo
> +  key = foo
> +  key = foo
> +  key = foo
> +EOF
> +	cat 5to1 5to1 5to1 5to1 5to1 >5to2 &&	   # 25
> +	cat 5to2 5to2 5to2 5to2 5to2 >5to3 &&	   # 125
> +	cat 5to3 5to3 5to3 5to3 5to3 >5to4 &&	   # 635
> +	cat 5to4 5to4 5to4 5to4 5to4 >>.git/config # 3125
> +}

You could make this even more semi-efficient by just storing the end
result in 5to5, and then copying it into place rather than doing the
whole dance for each test. I doubt it matters that much, though.

-Peff

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] config: arbitrary number of matches for --unset and --replace-all
  2013-11-14  8:37       ` Jeff King
@ 2013-11-14 20:24         ` Thomas Rast
  0 siblings, 0 replies; 8+ messages in thread
From: Thomas Rast @ 2013-11-14 20:24 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Junio C Hamano, Eric Sunshine, Jess Hottenstein

Jeff King <peff@peff.net> writes:

> This code is weird to follow because of the fall-throughs. I do not
> think you have introduced any bugs with your patch, but it seems weird
> to me that we set the offset at the top of the hunk. If we hit the
> conditional in the bottom half, we do actually increment storer.seen,
> but only _after_ having overwritten the value from above (with the same
> value, no less).
>
> But if we do not follow that code path, we may end up here:
>
>> @@ -1272,6 +1275,9 @@ static int store_aux(const char *key, const char *value, void *cb)
>>  			if (strrchr(key, '.') - key == store.baselen &&
>>  			      !strncmp(key, store.key, store.baselen)) {
>>  					store.state = SECTION_SEEN;
>> +					ALLOC_GROW(store.offset,
>> +						   store.seen+1,
>> +						   store.offset_alloc);
>>  					store.offset[store.seen] = cf->do_ftell(cf);
>>  			}
>>  		}
>
> where we overwrite it again, but do not update store.seen. Or we may
> trigger neither, and leave the function with our offset stored, but
> store.seen not incremented.

It's doubly strange that we write in this hunk without any protection
against overflow.  I was too lazy to think about it long enough to come
up with a possible example that triggers this, and instead just put in
the defensive ALLOC_GROW().  But if you can trigger it, it will probably
cause the algorithm to go off the rails because it overwrote store.state
and possibly even store.seen.

-- 
Thomas Rast
tr@thomasrast.ch

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH] fixup! config: arbitrary number of matches for --unset and --replace-all
  2013-11-13 10:19 ` [PATCH] config: arbitrary number of matches for --unset and --replace-all Thomas Rast
  2013-11-13 23:22   ` Eric Sunshine
@ 2013-12-06 18:46   ` Junio C Hamano
  2013-12-06 21:10     ` Jeff King
  1 sibling, 1 reply; 8+ messages in thread
From: Junio C Hamano @ 2013-12-06 18:46 UTC (permalink / raw)
  To: git; +Cc: Thomas Rast, Jeff King

---
 * I'll squash this to tr/config-multivalue-lift-max in preparation
   for merging it to 'master',which should happen by the end of
   this week.

   Thanks.

 config.c                |  8 ++++----
 t/t1303-wacky-config.sh | 14 +++++++-------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/config.c b/config.c
index 431dcb7..a1e80da 100644
--- a/config.c
+++ b/config.c
@@ -1192,7 +1192,7 @@ static int store_aux(const char *key, const char *value, void *cb)
 				warning("%s has multiple values", key);
 			}
 
-			ALLOC_GROW(store.offset, store.seen+1,
+			ALLOC_GROW(store.offset, store.seen + 1,
 				   store.offset_alloc);
 
 			store.offset[store.seen] = cf->do_ftell(cf);
@@ -1222,14 +1222,14 @@ static int store_aux(const char *key, const char *value, void *cb)
 		 * Do not increment matches: this is no match, but we
 		 * just made sure we are in the desired section.
 		 */
-		ALLOC_GROW(store.offset, store.seen+1,
+		ALLOC_GROW(store.offset, store.seen + 1,
 			   store.offset_alloc);
 		store.offset[store.seen] = cf->do_ftell(cf);
 		/* fallthru */
 	case SECTION_END_SEEN:
 	case START:
 		if (matches(key, value)) {
-			ALLOC_GROW(store.offset, store.seen+1,
+			ALLOC_GROW(store.offset, store.seen + 1,
 				   store.offset_alloc);
 			store.offset[store.seen] = cf->do_ftell(cf);
 			store.state = KEY_SEEN;
@@ -1239,7 +1239,7 @@ static int store_aux(const char *key, const char *value, void *cb)
 			      !strncmp(key, store.key, store.baselen)) {
 					store.state = SECTION_SEEN;
 					ALLOC_GROW(store.offset,
-						   store.seen+1,
+						   store.seen + 1,
 						   store.offset_alloc);
 					store.offset[store.seen] = cf->do_ftell(cf);
 			}
diff --git a/t/t1303-wacky-config.sh b/t/t1303-wacky-config.sh
index 7d55730..3a2c819 100755
--- a/t/t1303-wacky-config.sh
+++ b/t/t1303-wacky-config.sh
@@ -66,13 +66,13 @@ setup_many() {
 	# Semi-efficient way of concatenating 5^5 = 3125 lines. Note
 	# that because 'setup' already put one line, this means 3126
 	# entries for section.key in the config file.
-	cat >5to1 <<EOF
-  key = foo
-  key = foo
-  key = foo
-  key = foo
-  key = foo
-EOF
+	cat >5to1 <<-\EOF &&
+	  key = foo
+	  key = foo
+	  key = foo
+	  key = foo
+	  key = foo
+	EOF
 	cat 5to1 5to1 5to1 5to1 5to1 >5to2 &&	   # 25
 	cat 5to2 5to2 5to2 5to2 5to2 >5to3 &&	   # 125
 	cat 5to3 5to3 5to3 5to3 5to3 >5to4 &&	   # 635
-- 
1.8.5.1-402-gdd8f092

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] fixup! config: arbitrary number of matches for --unset and --replace-all
  2013-12-06 18:46   ` [PATCH] fixup! " Junio C Hamano
@ 2013-12-06 21:10     ` Jeff King
  0 siblings, 0 replies; 8+ messages in thread
From: Jeff King @ 2013-12-06 21:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Thomas Rast

On Fri, Dec 06, 2013 at 10:46:42AM -0800, Junio C Hamano wrote:

> ---
>  * I'll squash this to tr/config-multivalue-lift-max in preparation
>    for merging it to 'master',which should happen by the end of
>    this week.

Yeah, all makes sense to me. Thanks.

-Peff

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2013-12-06 21:10 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-05 15:31 Bug with git svn fetch? "error:too many matches for svn-remote.svn.added-placeholder" Jess Hottenstein
2013-11-13 10:19 ` [PATCH] config: arbitrary number of matches for --unset and --replace-all Thomas Rast
2013-11-13 23:22   ` Eric Sunshine
2013-11-14  7:50     ` Thomas Rast
2013-11-14  8:37       ` Jeff King
2013-11-14 20:24         ` Thomas Rast
2013-12-06 18:46   ` [PATCH] fixup! " Junio C Hamano
2013-12-06 21:10     ` Jeff King

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).