Re: [PATCH v2 4/7] grep: add support for the PCRE v1 JIT API

[Simon Ruderich <simon@ruderich.org>]

On Sat, May 13, 2017 at 11:45:32PM +0000, Ævar Arnfjörð Bjarmason wrote:
> [snip]
>
> +#ifdef PCRE_CONFIG_JIT
> +	if (p->pcre1_jit_on)
> +		ret = pcre_jit_exec(p->pcre1_regexp, p->pcre1_extra_info, line,
> +				    eol - line, 0, flags, ovector,
> +				    ARRAY_SIZE(ovector), p->pcre1_jit_stack);
> +	else
> +		ret = pcre_exec(p->pcre1_regexp, p->pcre1_extra_info, line,
> +				eol - line, 0, flags, ovector,
> +				ARRAY_SIZE(ovector));
> +#else
>  	ret = pcre_exec(p->pcre1_regexp, p->pcre1_extra_info, line, eol - line,
>  			0, flags, ovector, ARRAY_SIZE(ovector));
> +#endif

Wouldn't it be simpler to remove the duplication and
unconditionally use the old pcre_exec() call? Something like
this:

    +#ifdef PCRE_CONFIG_JIT
    +	if (p->pcre1_jit_on)
    +		ret = pcre_jit_exec(p->pcre1_regexp, p->pcre1_extra_info, line,
    +				    eol - line, 0, flags, ovector,
    +				    ARRAY_SIZE(ovector), p->pcre1_jit_stack);
    +	else
    +#endif
     	ret = pcre_exec(p->pcre1_regexp, p->pcre1_extra_info, line, eol - line,
     			0, flags, ovector, ARRAY_SIZE(ovector));

>  	if (ret < 0 && ret != PCRE_ERROR_NOMATCH)
>  		die("pcre_exec failed with error code %d", ret);
>  	if (ret > 0) {
> @@ -394,7 +420,16 @@ static int pcre1match(struct grep_pat *p, const char *line, const char *eol,
>  static void free_pcre1_regexp(struct grep_pat *p)
>  {
>  	pcre_free(p->pcre1_regexp);
> +#ifdef PCRE_CONFIG_JIT
> +	if (p->pcre1_jit_on) {
> +		pcre_free_study(p->pcre1_extra_info);
> +		pcre_jit_stack_free(p->pcre1_jit_stack);
> +	} else {
> +		pcre_free(p->pcre1_extra_info);
> +	}
> +#else
>  	pcre_free(p->pcre1_extra_info);
> +#endif

Same here. The pcre_free() is the same with and without the
ifdef.

Regards
Simon
-- 
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9