Ramsay Jones [off-list ref] writes:
quoted hunk
I think that, after some testing, this (or something like it) is the
best that we can do. What do you think?
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 <redacted>
---
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
+'
OK.
quoted hunk
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);
+}
Looks like a bit of overkill for only two call sites, whose output
we would never see because it is behind the test, but OK.
+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);
This could be the third call site of die() that tells the user to
build with NO_REGEX=1. Then "cd t && sh t0070-fundamental.sh -i -v" would
give that message directly to the user.
+ exit(0);
+}
Thanks.
Junio C Hamano wrote:
Ramsay Jones [off-list ref] writes:
[snip]
quoted
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);
+}
Looks like a bit of overkill for only two call sites, whose output
we would never see because it is behind the test, but OK.
Yes, there was a net increase in the line count when I introduced
die(), but the main program flow was less cluttered by error handling.
The net result looked much better, so I thought it was worth it.
What may not be too obvious, however, is that test-regex.c was written
to be independent of git. You should be able to compile the (single) file
on any POSIX system to determine if the system regex routines suffer this
problem. (It was also supposed to be quiet, unless it die()-ed, and
provide the result via the exit code).
Given that I'm now building it as part of git, I should have simply
#included <git-compat-util.h> and used the die() routine from libgit.a
(since I'm now *relying* on test-regex being linked with libgit.a).
quoted
+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);
This could be the third call site of die() that tells the user to
build with NO_REGEX=1. Then "cd t && sh t0070-fundamental.sh -i -v" would
give that message directly to the user.
Hmm, even without "-i -v", it's *very* clear what is going on, but sure
it wouldn't hurt either. (Also, I wanted to be able to distinguish an exit
via die() from a "test failed" error return).
So, new (tested) version of the patch comming.
ATB,
Ramsay Jones