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

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 8, 2016, 17:09 UTC
Message-ID: <xmqq1t0ujp2f.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <d0537819a3676fda6928e7ad3282aa71643f0755.1473321437.git.johannes.schindelin@gmx.de>

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

Show 11 quoted lines

> @@ -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.

Show 10 quoted lines

> @@ -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.