From: Thomas Rast <tr@thomasrast.ch>
To: git@vger.kernel.org
Cc: Jeff King <peff@peff.net>, Junio C Hamano <gitster@pobox.com>
Subject: [PATCH] config: arbitrary number of matches for --unset and --replace-all
Date: Wed, 13 Nov 2013 11:19:00 +0100 [thread overview]
Message-ID: <06ba1524cbe4fa31b6e1a8d644882521aeaff4f4.1384337608.git.tr@thomasrast.ch> (raw)
In-Reply-To: <CAB8C745oJjw6pZ1MFy73Wy=WM-8n=aRY7VUh73u__VLB5e8mQA@mail.gmail.com>
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
next prev parent reply other threads:[~2013-11-13 10:19 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2013-11-13 23:22 ` [PATCH] config: arbitrary number of matches for --unset and --replace-all 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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=06ba1524cbe4fa31b6e1a8d644882521aeaff4f4.1384337608.git.tr@thomasrast.ch \
--to=tr@thomasrast.ch \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).