On Thu, Jan 28, 2016 at 06:56:16PM +0700, Nguyễn Thái Ngọc Duy wrote:
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
> ---
> diff --git a/test-regex.c b/test-regex.c
> @@ -1,19 +1,63 @@
> int main(int argc, char **argv)
> {
> - char *pat = "[^={} \t]+";
> - char *str = "={}\nfred";
> + const char *pat;
> + const char *str;
> + int flags = 0;
> regex_t r;
> regmatch_t m[1];
>
> - if (regcomp(&r, pat, REG_EXTENDED | REG_NEWLINE))
> + if (argc == 1) {
> + /* special case, bug check */
> + pat = "[^={} \t]+";
> + str = "={}\nfred";
> + flags = REG_EXTENDED | REG_NEWLINE;
> + } else {
> + argv++;
> + pat = *argv++;
> + str = *argv++;
I realize that this is just a test program, but it might be a good
idea to insert:
if (argc < 3)
die("usage: ...");
prior to the *argv++ dereferences to give a controlled failure rather
than an outright crash when an incorrect number of arguments is
given.
More below...
> + while (*argv) {
> + struct reg_flag *rf;
> + for (rf = reg_flags; rf->name; rf++)
> + if (!strcmp(*argv, rf->name)) {
> + flags |= rf->flag;
> + break;
> + }
> + if (!rf->name)
> + die("do not recognize %s", *argv);
> + argv++;
> + }
> + git_setup_gettext();
> + }
> +
> + if (regcomp(&r, pat, flags))
> 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);
> + if (regexec(&r, str, 1, m, 0)) {
> + if (argc == 1)
> + die("no match of pattern '%s' to string '%s'", pat, str);
> + return 1;
> + }
>
> /* http://sourceware.org/bugzilla/show_bug.cgi?id=3957 */
> - if (m[0].rm_so == 3) /* matches '\n' when it should not */
> + if (argc == 1 && m[0].rm_so == 3) /* matches '\n' when it should not */
> die("regex bug confirmed: re-build git with NO_REGEX=1");
Again, I realize that this is just a test program, but sprinkling
this 'argc == 1' special case throughout the code makes it
unnecessarily difficult to follow. Some alternatives:
1. Rename the existing test-regex to test-regex-bug (or
test-regex-bugs), and then name the new general purpose program
test-regex.
2. Drop the special case altogether and have the program emit the
matched text on stdout (in addition to the exit code indicating
success/failure). Most callers will care only about the exit
status, but the one special case in t0070 which wants to check for
the glibc bug can do so itself:
test_expect_success 'check for a bug in the regex routines' '
# if this test fails, re-build git with NO_REGEX=1
printf "fred" >expect &&
test-regex "[^={} \t]+" "={}\nfred" EXTENDED NEWLINE >actual &&
test_cmp expect actual
'
Of course, that doesn't actually work because "\n" in the 'str'
argument isn't really a newline, so test-regex would have to do a
bit of preprocessing of 'str' first (which might be as simple as
calling unquote_c_style() or something).
3. [less desirable] Move the 'argc == 1' special case to its own
function, which will result in a bit of duplicated code, but the
result should at least be easier to follow.
> exit(0);
> --
> 2.7.0.288.g1d8ad15
--
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