Re: [igt-dev] [PATCH i-g-t 1/3] runner: Reinitialize compiled dmesg regexp each parsing session

["Ser, Simon" <simon.ser@intel.com>]

On Mon, 2019-04-01 at 09:46 +0300, Arkadiusz Hiler wrote:
> Which regexp gets compiled is settings specific, depending whether we
> run piglit-style or not.
> 
> If it's optimized to be initialized only once and it is a global
> variable, it will be "stuck" in the mode we have selected with the
> first
> run, which may break tests.
> 
> Let's remove this optimization and initialize it each time, as it takes
> less 0.002s on my hardware.
> 
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> ---
>  runner/resultgen.c | 29 +++++++++++------------------
>  1 file changed, 11 insertions(+), 18 deletions(-)
> 
> diff --git a/runner/resultgen.c b/runner/resultgen.c
> index 32b59d59..73fda64f 100644
> --- a/runner/resultgen.c
> +++ b/runner/resultgen.c
> @@ -499,27 +499,18 @@ static const char igt_dmesg_whitelist[] =
>  static const char igt_piglit_style_dmesg_blacklist[] =
>  	"(\\[drm:|drm_|intel_|i915_)";
>  
> -static regex_t re;
> -
> -static int init_regex_whitelist(struct settings *settings)
> +static bool init_regex_whitelist(struct settings *settings, regex_t* re)

Style nit: the * should be on the right

Otherwise, this is:

Reviewed-by: Simon Ser <simon.ser@intel.com>

>  {
> -	static int status = -1;
> +	const char *regex = settings->piglit_style_dmesg ?
> +		igt_piglit_style_dmesg_blacklist :
> +		igt_dmesg_whitelist;
>  
> -	if (status == -1) {
> -		const char *regex = settings->piglit_style_dmesg ?
> -			igt_piglit_style_dmesg_blacklist :
> -			igt_dmesg_whitelist;
> -
> -		if (regcomp(&re, regex, REG_EXTENDED | REG_NOSUB) != 0)
> {
> -			fprintf(stderr, "Cannot compile dmesg
> regexp\n");
> -			status = 1;
> -			return false;
> -		}
> -
> -		status = 0;
> +	if (regcomp(re, regex, REG_EXTENDED | REG_NOSUB) != 0) {
> +		fprintf(stderr, "Cannot compile dmesg regexp\n");
> +		return false;
>  	}
>  
> -	return status;
> +	return true;
>  }
>  
>  static bool parse_dmesg_line(char* line,
> @@ -639,12 +630,13 @@ static bool fill_from_dmesg(int fd,
>  	char piglit_name[256];
>  	ssize_t read;
>  	size_t i;
> +	regex_t re;
>  
>  	if (!f) {
>  		return false;
>  	}
>  
> -	if (init_regex_whitelist(settings)) {
> +	if (!init_regex_whitelist(settings, &re)) {
>  		fclose(f);
>  		return false;
>  	}
> @@ -723,6 +715,7 @@ static bool fill_from_dmesg(int fd,
>  
>  	free(dmesg);
>  	free(warnings);
> +	regfree(&re);
>  	fclose(f);
>  	return true;
>  }
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev