ACK: [PATCH 1/2] lib: klog: remove use of pcre, use regex instead
Alex Hung
alex.hung at canonical.com
Wed Nov 25 03:41:39 UTC 2015
On 2015-11-24 09:01 PM, Colin King wrote:
> From: Colin Ian King <colin.king at canonical.com>
>
> Using pcre does not offer any extra benefit over regex and
> requires an extra library dependency. Remove pcre and replace
> with regex. Tested on i386 and amd64 Ubuntu releases of
> precise, trusty, vivid, wily and xenial.
>
> Signed-off-by: Colin Ian King <colin.king at canonical.com>
> ---
> debian/control | 2 +-
> src/lib/include/fwts_klog.h | 6 +--
> src/lib/src/Makefile.am | 2 +-
> src/lib/src/fwts_klog.c | 99 +++++++++++++--------------------------------
> 4 files changed, 32 insertions(+), 77 deletions(-)
>
> diff --git a/debian/control b/debian/control
> index e63f272..52e2b8f 100644
> --- a/debian/control
> +++ b/debian/control
> @@ -4,7 +4,7 @@ Priority: optional
> Maintainer: Firmware Testing Team <fwts-devel at lists.ubuntu.com>
> Uploaders: Colin King <colin.king at ubuntu.com>, Keng-Yu Lin <kengyu at ubuntu.com>, Alex Hung <alex.hung at canonical.com>, Ivan Hu <ivan.hu at canonical.com>
> Standards-Version: 3.9.5
> -Build-Depends: debhelper (>= 7.0.50~), autoconf, automake, libtool, libpcre3-dev (>= 7.8), libjson0-dev (>= 0.9), flex, bison, hardening-wrapper, dh-autoreconf, dkms, libglib2.0-dev, pkg-config
> +Build-Depends: debhelper (>= 7.0.50~), autoconf, automake, libtool, libjson0-dev (>= 0.9), flex, bison, hardening-wrapper, dh-autoreconf, dkms, libglib2.0-dev, pkg-config
>
> Package: fwts
> Architecture: i386 amd64 armel armhf arm64 ppc64 ppc64el
> diff --git a/src/lib/include/fwts_klog.h b/src/lib/include/fwts_klog.h
> index cc56258..847cde9 100644
> --- a/src/lib/include/fwts_klog.h
> +++ b/src/lib/include/fwts_klog.h
> @@ -21,7 +21,7 @@
> #define __FWTS_KLOG_H__
>
> #include <sys/types.h>
> -#include <pcre.h>
> +#include <regex.h>
>
> #include "fwts_list.h"
> #include "fwts_framework.h"
> @@ -43,8 +43,8 @@ typedef struct {
> const char *pattern;
> const char *advice;
> char *label;
> - pcre *re;
> - pcre_extra *extra;
> + regex_t compiled;
> + bool compiled_ok;
> } fwts_klog_pattern;
>
> typedef void (*fwts_klog_progress_func)(fwts_framework *fw, int percent);
> diff --git a/src/lib/src/Makefile.am b/src/lib/src/Makefile.am
> index f3abde8..c16f129 100644
> --- a/src/lib/src/Makefile.am
> +++ b/src/lib/src/Makefile.am
> @@ -11,7 +11,7 @@ AM_CPPFLAGS = \
> pkglib_LTLIBRARIES = libfwts.la
>
> libfwts_la_LDFLAGS = \
> - -lm -lpcre -lpthread \
> + -lm -lpthread \
> -version-info 1:0:0 \
> -L$(top_builddir)/src/acpica/source/compiler \
> -lfwtsiasl `pkg-config --libs glib-2.0 gio-2.0`
> diff --git a/src/lib/src/fwts_klog.c b/src/lib/src/fwts_klog.c
> index 9c51b32..c164396 100644
> --- a/src/lib/src/fwts_klog.c
> +++ b/src/lib/src/fwts_klog.c
> @@ -22,10 +22,10 @@
> #include <stdlib.h>
> #include <stdbool.h>
> #include <sys/types.h>
> -#include <pcre.h>
> #include <ctype.h>
> #include <sys/types.h>
> #include <sys/stat.h>
> +#include <regex.h>
> #include <fcntl.h>
>
> #include "fwts.h"
> @@ -34,7 +34,6 @@
> * klog pattern matching strings data file, data stored in json format
> */
> #define KLOG_DATA_JSON_FILE "klog.json"
> -#define VECTOR_SIZE (3) /* Must be a multiple of 3 */
>
> /*
> * fwts_klog_free()
> @@ -269,7 +268,6 @@ void fwts_klog_scan_patterns(fwts_framework *fw,
> int *errors)
> {
> fwts_klog_pattern *pattern = (fwts_klog_pattern *)private;
> - int vector[VECTOR_SIZE];
> static char *advice =
> "This is a bug picked up by the kernel, but as yet, the "
> "firmware test suite has no diagnostic advice for this particular problem.";
> @@ -280,43 +278,16 @@ void fwts_klog_scan_patterns(fwts_framework *fw,
> bool matched = false;
> switch (pattern->compare_mode) {
> case FWTS_COMPARE_REGEX:
> - if (pattern->re) {
> - int ret = pcre_exec(pattern->re, pattern->extra, line, strlen(line), 0, 0, vector, VECTOR_SIZE);
> - if (ret < 0) {
> - char *errmsg = NULL;
> - /*
> - * We should handle -ve conditions other
> - * than PCRE_ERROR_NOMATCH as pcre internal
> - * errors..
> - */
> - switch (ret) {
> - case PCRE_ERROR_NOMATCH:
> - break;
> - case PCRE_ERROR_NULL:
> - errmsg = "internal NULL error";
> - break;
> - case PCRE_ERROR_BADOPTION:
> - errmsg = "invalid option passed to pcre_exec()";
> - break;
> - case PCRE_ERROR_BADMAGIC:
> - errmsg = "bad magic number, (corrupt regular expression)";
> - break;
> - case PCRE_ERROR_UNKNOWN_NODE:
> - errmsg = "compiled regular expression broken";
> - break;
> - case PCRE_ERROR_NOMEMORY:
> - errmsg = "out of memory";
> - break;
> - default:
> - errmsg = "unknown error";
> - break;
> - }
> - if (errmsg)
> - fwts_log_info(fw, "regular expression engine error: %s.", errmsg);
> - }
> - else {
> + if (pattern->compiled_ok) {
> + int ret = regexec(&pattern->compiled, line, 0, NULL, 0);
> + if (!ret) {
> /* A successful regular expression match! */
> matched = true;
> + } else if (ret != REG_NOMATCH) {
> + char msg[1024];
> +
> + regerror(ret, &pattern->compiled, msg, sizeof(msg));
> + fwts_log_info(fw, "regular expression engine error: %s.", msg);
> }
> }
> break;
> @@ -452,8 +423,6 @@ static int fwts_klog_check(fwts_framework *fw,
>
> /* Now fetch json objects and compile regex */
> for (i = 0; i < n; i++) {
> - const char *error;
> - int erroffset;
> const char *str;
> json_object *obj;
>
> @@ -488,21 +457,15 @@ static int fwts_klog_check(fwts_framework *fw,
> goto fail;
>
> if (patterns[i].compare_mode == FWTS_COMPARE_REGEX) {
> - if ((patterns[i].re = pcre_compile(patterns[i].pattern, 0, &error, &erroffset, NULL)) == NULL) {
> - fwts_log_error(fw, "Regex %s failed to compile: %s.", patterns[i].pattern, error);
> - patterns[i].re = NULL;
> - patterns[i].extra = NULL;
> + int rc;
> +
> + rc = regcomp(&patterns[i].compiled, patterns[i].pattern, REG_EXTENDED);
> + if (rc) {
> + fwts_log_error(fw, "Regex %s failed to compile: %d.", patterns[i].pattern, rc);
> + patterns[i].compiled_ok = false;
> } else {
> - patterns[i].extra = pcre_study(patterns[i].re, 0, &error);
> - if (error != NULL) {
> - fwts_log_error(fw, "Regex %s failed to optimize: %s.", patterns[i].pattern, error);
> - patterns[i].re = NULL;
> - patterns[i].extra = NULL;
> - }
> + patterns[i].compiled_ok = true;
> }
> - } else {
> - patterns[i].re = NULL;
> - patterns[i].extra = NULL;
> }
> }
> /* We've now collected up the scan patterns, lets scan the log for errors */
> @@ -510,10 +473,8 @@ static int fwts_klog_check(fwts_framework *fw,
>
> fail:
> for (i = 0; i < n; i++) {
> - if (patterns[i].re)
> - pcre_free(patterns[i].re);
> - if (patterns[i].extra)
> - pcre_free(patterns[i].extra);
> + if (patterns[i].compiled_ok)
> + regfree(&patterns[i].compiled);
> if (patterns[i].label)
> free(patterns[i].label);
> }
> @@ -541,26 +502,20 @@ int fwts_klog_pm_check(fwts_framework *fw, fwts_klog_progress_func progress,
> static void fwts_klog_regex_find_callback(fwts_framework *fw, char *line, int repeated,
> char *prev, void *pattern, int *match)
> {
> - const char *error;
> - int erroffset;
> - pcre *re;
> + int rc;
> + regex_t compiled;
>
> FWTS_UNUSED(fw);
> FWTS_UNUSED(repeated);
> FWTS_UNUSED(prev);
>
> - re = pcre_compile(pattern, 0, &error, &erroffset, NULL);
> - if (re != NULL) {
> - int rc;
> - int vector[VECTOR_SIZE];
> - pcre_extra *extra = pcre_study(re, 0, &error);
> -
> - if (error)
> - return;
> - rc = pcre_exec(re, extra, line, strlen(line), 0, 0, vector, VECTOR_SIZE);
> - free(extra);
> - pcre_free(re);
> - if (rc == 0)
> + rc = regcomp(&compiled, (char *)pattern, REG_EXTENDED);
> + if (rc) {
> + fwts_log_error(fw, "Regex %s failed to compile: %d.", (char *)pattern, rc);
> + } else {
> + rc = regexec(&compiled, line, 0, NULL, 0);
> + regfree(&compiled);
> + if (!rc)
> (*match)++;
> }
> }
>
Acked-by: Alex Hung <alex.hung at canonical.com>
More information about the fwts-devel
mailing list