From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 09 Feb 2016 18:21:28 GMT
Subject: Re: [PATCH v6 04/11] test-regex: expose full regcomp() to the command line
Message-ID: <xmqq8u2t4udz.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAPig+cQ2yFvM_YeeN+OObo+jZaxtiaLi+URE3USB2v4fUyC9tw@mail.gmail.com>

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Fri, Feb 5, 2016 at 9:03 PM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>> diff --git a/test-regex.c b/test-regex.c
>> @@ -21,8 +38,38 @@ static int test_regex_bug(void)
>>  int main(int argc, char **argv)
>>  {
>> +       const char *pat;
>> +       const char *str;
>> +       int flags = 0;
>> +       regex_t r;
>> +       regmatch_t m[1];
>> +
>>         if (argc == 2 && !strcmp(argv[1], "--bug"))
>>                 return test_regex_bug();
>> -       else
>> -               die("must be used with --bug");
>> +       else if (argc < 3)
>> +               die("usage: test-regex --bug\n"
>> +                   "       test-regex <pattern> <string> [<options>]");
>
> This is just a test program, so it probably isn't that important, but
> die() automatically prepends "fatal: " which means the alignment of
> the second line will be wrong. Perhaps you want to use usage() instead
> which automatically prepends "usage: " (and drop the literal "usage: "
> from the above string).

So true.

>> +       argv++;
>> +       pat = *argv++;
>> +       str = *argv++;
>> +       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))
>> +               return 1;
>> +
>> +       return 0;
>>  }
>
> This version is much easier to read without the "bug" special case
> spread throughout the code. Thanks.