Re: [igt-dev] [PATCH i-g-t] runner: Use glib's regex utilities instead of POSIX ERE

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

On Thu, 2019-05-09 at 20:06 -0400, Lyude wrote:
> From: Lyude Paul <lyude@redhat.com>
> 
> POSIX ERE, while pretty standard is also very old and severely limited.
> In fact, it's so limited that Intel CI's own test blacklist,
> tests/intel-ci/blacklist.txt, doesn't even work with it due to the lack
> of support for backreferences.
> 
> Since we're already using glib in other parts of igt, let's just start
> using glib's regular expression matching instead of the POSIX regex API.
> This gives us full perl compatible regular expressions and in turn, also
> gives us python compatible regular expressions to match piglit.

Side note: a more portable alternative would be PCRE.

> Signed-off-by: Lyude Paul <lyude@redhat.com>
> ---
>  runner/job_list.c  |  4 +---
>  runner/meson.build |  2 +-
>  runner/resultgen.c | 15 +++++++++------
>  runner/settings.c  | 26 ++++++++++----------------
>  runner/settings.h  |  4 ++--
>  5 files changed, 23 insertions(+), 28 deletions(-)
> 
> diff --git a/runner/job_list.c b/runner/job_list.c
> index 941e2ee0..4a16742f 100644
> --- a/runner/job_list.c
> +++ b/runner/job_list.c
> @@ -17,10 +17,8 @@ static bool matches_any(const char *str, struct regex_list *list)
>  	size_t i;
>  
>  	for (i = 0; i < list->size; i++) {
> -		if (regexec(list->regexes[i], str,
> -			    (size_t)0, NULL, 0) == 0) {
> +		if (g_regex_match(list->regexes[i], str, 0, NULL))
>  			return true;
> -		}
>  	}
>  
>  	return false;
> diff --git a/runner/meson.build b/runner/meson.build
> index de6e6f1c..1ffc95b4 100644
> --- a/runner/meson.build
> +++ b/runner/meson.build
> @@ -17,7 +17,7 @@ if _build_runner and jsonc.found()
>  
>  	runnerlib = static_library('igt_runner', runnerlib_sources,
>  				   include_directories : inc,
> -				   dependencies : jsonc)
> +				   dependencies : [jsonc, glib])
>  
>  	runner = executable('igt_runner', runner_sources,
>  			    link_with : runnerlib,
> diff --git a/runner/resultgen.c b/runner/resultgen.c
> index d9702a19..2b7d26d5 100644
> --- a/runner/resultgen.c
> +++ b/runner/resultgen.c
> @@ -499,14 +499,17 @@ static const char igt_dmesg_whitelist[] =
>  static const char igt_piglit_style_dmesg_blacklist[] =
>  	"(\\[drm:|drm_|intel_|i915_)";
>  
> -static bool init_regex_whitelist(struct settings* settings, regex_t* re)
> +static bool init_regex_whitelist(struct settings* settings, GRegex **re)
>  {
> +	GError *err = NULL;
>  	const char *regex = settings->piglit_style_dmesg ?
>  		igt_piglit_style_dmesg_blacklist :
>  		igt_dmesg_whitelist;
>  
> -	if (regcomp(re, regex, REG_EXTENDED | REG_NOSUB) != 0) {
> +	*re = g_regex_new(regex, G_REGEX_OPTIMIZE, 0, &err);
> +	if (err) {
>  		fprintf(stderr, "Cannot compile dmesg regexp\n");
> +		g_error_free(err);
>  		return false;
>  	}
>  
> @@ -630,7 +633,7 @@ static bool fill_from_dmesg(int fd,
>  	char piglit_name[256];
>  	ssize_t read;
>  	size_t i;
> -	regex_t re;
> +	GRegex *re;
>  
>  	if (!f) {
>  		return false;
> @@ -671,12 +674,12 @@ static bool fill_from_dmesg(int fd,
>  
>  		if (settings->piglit_style_dmesg) {
>  			if ((flags & 0x07) <= settings->dmesg_warn_level && continuation != 'c' &&
> -			    regexec(&re, message, (size_t)0, NULL, 0) != REG_NOMATCH) {
> +			    g_regex_match(re, message, 0, NULL)) {
>  				append_line(&warnings, &warningslen, formatted);
>  			}
>  		} else {
>  			if ((flags & 0x07) <= settings->dmesg_warn_level && continuation != 'c' &&
> -			    regexec(&re, message, (size_t)0, NULL, 0) == REG_NOMATCH) {
> +			    !g_regex_match(re, message, 0, NULL)) {
>  				append_line(&warnings, &warningslen, formatted);
>  			}
>  		}
> @@ -715,7 +718,7 @@ static bool fill_from_dmesg(int fd,
>  
>  	free(dmesg);
>  	free(warnings);
> -	regfree(&re);
> +	g_regex_unref(re);
>  	fclose(f);
>  	return true;
>  }
> diff --git a/runner/settings.c b/runner/settings.c
> index 25bcf531..ad38ae8d 100644
> --- a/runner/settings.c
> +++ b/runner/settings.c
> @@ -187,23 +187,18 @@ static void usage(const char *extra_message, FILE *f)
>  
>  static bool add_regex(struct regex_list *list, char *new)
>  {
> -	regex_t *regex;
> -	size_t buflen;
> -	char *buf;
> -	int s;
> -
> -	regex = malloc(sizeof(*regex));
> -
> -	if ((s = regcomp(regex, new,
> -			 REG_EXTENDED | REG_NOSUB)) != 0) {
> -		buflen = regerror(s, regex, NULL, 0);
> -		buf = malloc(buflen);
> -		regerror(s, regex, buf, buflen);
> +	GRegex *regex;
> +	GError *error = NULL;
> +
> +	regex = g_regex_new(new, G_REGEX_OPTIMIZE, 0, &error);
> +	if (error) {
> +		char *buf = malloc(snprintf(NULL, 0, "Invalid regex '%s': %s", new, error->message) + 1);
> +
> +		sprintf(buf, "Invalid regex '%s': %s", new, error->message);
>  		usage(buf, stderr);
>  
>  		free(buf);
> -		regfree(regex);
> -		free(regex);
> +		g_error_free(error);
>  		return false;
>  	}
>  
> @@ -224,8 +219,7 @@ static void free_regexes(struct regex_list *regexes)
>  
>  	for (i = 0; i < regexes->size; i++) {
>  		free(regexes->regex_strings[i]);
> -		regfree(regexes->regexes[i]);
> -		free(regexes->regexes[i]);
> +		g_regex_unref(regexes->regexes[i]);
>  	}
>  	free(regexes->regex_strings);
>  	free(regexes->regexes);
> diff --git a/runner/settings.h b/runner/settings.h
> index 672a3af8..0a1ee08d 100644
> --- a/runner/settings.h
> +++ b/runner/settings.h
> @@ -4,8 +4,8 @@
>  #include <stdbool.h>
>  #include <stddef.h>
>  #include <sys/types.h>
> -#include <regex.h>
>  #include <stdio.h>
> +#include <glib.h>
>  
>  enum {
>  	LOG_LEVEL_NORMAL = 0,
> @@ -21,7 +21,7 @@ _Static_assert(ABORT_ALL == (ABORT_TAINT | ABORT_LOCKDEP), "ABORT_ALL must be al
>  
>  struct regex_list {
>  	char **regex_strings;
> -	regex_t** regexes;
> +	GRegex **regexes;
>  	size_t size;
>  };
>  
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev