Re: [PATCH v3 3/3] Use the newly-introduced regexec_buf() function

[Junio C Hamano]

Johannes Schindelin <johannes.schinde...@gmx.de> writes:

> @@ -33,11 +32,8 @@ static void diffgrep_consume(void *priv, char *line, 
> unsigned long len)
>                * caller early.
>                */
>               return;
> -     /* Yuck -- line ought to be "const char *"! */
> -     hold = line[len];
> -     line[len] = '\0';
> -     data->hit = !regexec(data->regexp, line + 1, 1, &regmatch, 0);
> -     line[len] = hold;
> +     data->hit = !regexec_buf(data->regexp, line + 1, len - 1, 1,
> +                              &regmatch, 0);

This is an unexpected happy surprise.  It really feels good to see
that "Yuck" line go.

> @@ -228,18 +227,16 @@ static long ff_regexp(const char *line, long len,
>                       len--;
>       }
>  
> -     line_buffer = xstrndup(line, len); /* make NUL terminated */
> -
>       for (i = 0; i < regs->nr; i++) {
>               struct ff_reg *reg = regs->array + i;
> -             if (!regexec(&reg->re, line_buffer, 2, pmatch, 0)) {
> +             if (!regexec_buf(&reg->re, line, len, 2, pmatch, 0)) {

So is this hunk.  Removing unnecessary copying is a very good thing.

Please give these three patches a common prefix, e.g.

        regex: -G<pattern> feeds a non NUL-terminated string to regexec() and 
fails
        regex: add regexec_buf() that can work on a non NUL-terminated string
        regex: use regexec_buf()

or something like that.

Also I agree with Peff that a test with an embedded NUL would be a
good thing.

This round is so close to perfect.