Junio C Hamano wrote:
> Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxx> writes:
>
[snip]
>> 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).
>> +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
--
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