Jonathan Nieder wrote:
> Ramsay Jones wrote:
>> This is an RFC because:
>> - A simple fix would be for me to put NO_REGEX=1 in my config.mak,
>> since the compat/regex routines don't suffer this problem.
>> - I suspect this bug is old enough that it will not affect many users.
>> - I have not audited the other non-matching list expressions in
>> userdiff.c
>> - blame, grep and pickaxe all call regcomp() with the REG_NEWLINE
>> flag, but get the regex from the user (eg from command line).
>
> I think excluding \n along with ' ' and \t in similar places makes
> sense, ...
I briefly considered replacing ' \t' with [:space:], but that would
also add \v, \f and \r in addition to \n (and, maybe, some locale
specific chars too), so I decided not to go that route ...
> ... but that meanwhile we should add tests to the testsuite for
> blame/grep/pickaxe to catch such implementations failing when
> NO_REGEX=1 is not set.
Hmm, well I would certainly never complain about additional tests, but
who would these tests benefit? I suspect that the number of people
building git from source on an old (pre about 2008 say) glibc system
with this bug is somewhat small. (Of course I have no numbers to back
up that gut feeling! :-))
Having said that, I have attached a patch (below), that adds a test
(to t0070-fundamental.sh) using a stand-alone test-regex program, in
order to simplify the detection of this bug.
What do you think? (would this exchange in the list-archive suffice?)
> Does that make sense? Thanks for tracking this down.
No problem.
ATB,
Ramsay Jones
-- >8 --
Subject: [PATCH] test-regex: Add a test to check for a bug in the regex routines
Signed-off-by: Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxx>
---
.gitignore | 1 +
Makefile | 1 +
t/t0070-fundamental.sh | 5 +++++
test-regex.c | 35 +++++++++++++++++++++++++++++++++++
4 files changed, 42 insertions(+), 0 deletions(-)
create mode 100644 test-regex.c
diff --git a/.gitignore b/.gitignore
index 711fce7..70af8b1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -174,6 +174,7 @@
/test-obj-pool
/test-parse-options
/test-path-utils
+/test-regex
/test-run-command
/test-sha1
/test-sigchain
diff --git a/Makefile b/Makefile
index 3a1fe20..482b9f9 100644
--- a/Makefile
+++ b/Makefile
@@ -428,6 +428,7 @@ TEST_PROGRAMS_NEED_X += test-match-trees
TEST_PROGRAMS_NEED_X += test-obj-pool
TEST_PROGRAMS_NEED_X += test-parse-options
TEST_PROGRAMS_NEED_X += test-path-utils
+TEST_PROGRAMS_NEED_X += test-regex
TEST_PROGRAMS_NEED_X += test-run-command
TEST_PROGRAMS_NEED_X += test-sha1
TEST_PROGRAMS_NEED_X += test-sigchain
diff --git a/t/t0070-fundamental.sh b/t/t0070-fundamental.sh
index 9bee8bf..da2c504 100755
--- a/t/t0070-fundamental.sh
+++ b/t/t0070-fundamental.sh
@@ -25,4 +25,9 @@ test_expect_success POSIXPERM 'mktemp to unwritable directory prints filename' '
grep "cannotwrite/test" err
'
+test_expect_success 'check for a bug in the regex routines' '
+ # if this test fails, re-build git with NO_REGEX=1
+ test-regex
+'
+
test_done
diff --git a/test-regex.c b/test-regex.c
new file mode 100644
index 0000000..9259985
--- /dev/null
+++ b/test-regex.c
@@ -0,0 +1,35 @@
+#include <stdlib.h>
+#include <stdio.h>
+#include <stdarg.h>
+#include <sys/types.h>
+#include <regex.h>
+
+static void die(const char *fmt, ...)
+{
+ va_list p;
+
+ va_start(p, fmt);
+ vfprintf(stderr, fmt, p);
+ va_end(p);
+ fputc('\n', stderr);
+ exit(128);
+}
+
+int main(int argc, char **argv)
+{
+ char *pat = "[^={} \t]+";
+ char *str = "={}\nfred";
+ regex_t r;
+ regmatch_t m[1];
+
+ if (regcomp(&r, pat, REG_EXTENDED | REG_NEWLINE))
+ die("failed regcomp() for pattern '%s'", pat);
+ if (regexec(&r, str, 1, m, 0))
+ die("no match of pattern '%s' to string '%s'", pat, str);
+
+ /* http://sourceware.org/bugzilla/show_bug.cgi?id=3957 */
+ if (m[0].rm_so == 3) /* matches '\n' when it should not */
+ exit(1);
+
+ exit(0);
+}
--
1.7.5
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html