[libvirt] [PATCH 08/11] src: convert over to use GRegex for regular exprssions

Daniel P. Berrangé posted 11 patches 6 years, 4 months ago
Only 10 patches received!
There is a newer version of this series
[libvirt] [PATCH 08/11] src: convert over to use GRegex for regular exprssions
Posted by Daniel P. Berrangé 6 years, 4 months ago
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/conf/domain_event.c        | 25 ++++++++-----------
 src/libxl/libxl_capabilities.c | 44 ++++++++++++++++------------------
 2 files changed, 31 insertions(+), 38 deletions(-)

diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index b33589f472..f7d1a38e46 100644
--- a/src/conf/domain_event.c
+++ b/src/conf/domain_event.c
@@ -22,8 +22,6 @@
 
 #include <config.h>
 
-#include <regex.h>
-
 #include "domain_event.h"
 #include "object_event.h"
 #include "object_event_private.h"
@@ -2009,7 +2007,7 @@ virDomainQemuMonitorEventNew(int id,
  * deregisters.  */
 struct virDomainQemuMonitorEventData {
     char *event;
-    regex_t regex;
+    GRegex *regex;
     unsigned int flags;
     void *opaque;
     virFreeCallback freecb;
@@ -2241,7 +2239,7 @@ virDomainQemuMonitorEventFilter(virConnectPtr conn ATTRIBUTE_UNUSED,
     if (data->flags == -1)
         return true;
     if (data->flags & VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX)
-        return regexec(&data->regex, monitorEvent->event, 0, NULL, 0) == 0;
+        return g_regex_match(data->regex, monitorEvent->event, 0, NULL) == TRUE;
     if (data->flags & VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_NOCASE)
         return STRCASEEQ(monitorEvent->event, data->event);
     return STREQ(monitorEvent->event, data->event);
@@ -2255,7 +2253,7 @@ virDomainQemuMonitorEventCleanup(void *opaque)
 
     VIR_FREE(data->event);
     if (data->flags & VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX)
-        regfree(&data->regex);
+        g_regex_unref(data->regex);
     if (data->freecb)
         (data->freecb)(data->opaque);
     VIR_FREE(data);
@@ -2306,20 +2304,17 @@ virDomainQemuMonitorEventStateRegisterID(virConnectPtr conn,
         return -1;
     data->flags = flags;
     if (event && flags != -1) {
-        int rflags = REG_NOSUB;
-
-        if (flags & VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_NOCASE)
-            rflags |= REG_ICASE;
         if (flags & VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX) {
-            int err = regcomp(&data->regex, event, rflags);
+            int cflags = 0;
+            g_autoptr(GError) err = NULL;
 
-            if (err) {
-                char error[100];
-                regerror(err, &data->regex, error, sizeof(error));
+            if (flags & VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_NOCASE)
+                cflags |= G_REGEX_CASELESS;
+            data->regex = g_regex_new(event, cflags, 0, &err);
+            if (!data->regex) {
                 virReportError(VIR_ERR_INVALID_ARG,
                                _("failed to compile regex '%s': %s"),
-                               event, error);
-                regfree(&data->regex);
+                               event, err->message);
                 VIR_FREE(data);
                 return -1;
             }
diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c
index 73ae0b3fa1..90d9bac9c7 100644
--- a/src/libxl/libxl_capabilities.c
+++ b/src/libxl/libxl_capabilities.c
@@ -21,7 +21,6 @@
 #include <config.h>
 
 #include <libxl.h>
-#include <regex.h>
 
 #include "internal.h"
 #include "virlog.h"
@@ -374,10 +373,10 @@ static int
 libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps)
 {
     const libxl_version_info *ver_info;
-    int err;
-    regex_t regex;
+    g_autoptr(GRegex) regex = NULL;
+    g_autoptr(GError) err = NULL;
+    g_autoptr(GMatchInfo) info = NULL;
     char *str, *token;
-    regmatch_t subs[4];
     char *saveptr = NULL;
     size_t i;
 
@@ -398,12 +397,10 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps)
         return -1;
     }
 
-    err = regcomp(&regex, XEN_CAP_REGEX, REG_EXTENDED);
-    if (err != 0) {
-        char error[100];
-        regerror(err, &regex, error, sizeof(error));
+    regex = g_regex_new(XEN_CAP_REGEX, G_REGEX_EXTENDED, 0, &err);
+    if (!regex) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("Failed to compile regex %s"), error);
+                       _("Failed to compile regex %s"), err->message);
         return -1;
     }
 
@@ -436,31 +433,33 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps)
          nr_guest_archs < sizeof(guest_archs) / sizeof(guest_archs[0])
                  && (token = strtok_r(str, " ", &saveptr)) != NULL;
          str = NULL) {
-        if (regexec(&regex, token, sizeof(subs) / sizeof(subs[0]),
-                    subs, 0) == 0) {
-            int hvm = STRPREFIX(&token[subs[1].rm_so], "hvm");
+        if (g_regex_match(regex, token, 0, &info)) {
+            g_autofree char *modestr = g_match_info_fetch(info, 1);
+            g_autofree char *archstr = g_match_info_fetch(info, 2);
+            g_autofree char *suffixstr = g_match_info_fetch(info, 3);
+            int hvm = STRPREFIX(modestr, "hvm");
             virArch arch;
             int pae = 0, nonpae = 0, ia64_be = 0;
 
-            if (STRPREFIX(&token[subs[2].rm_so], "x86_32")) {
+            if (STRPREFIX(archstr, "x86_32")) {
                 arch = VIR_ARCH_I686;
-                if (subs[3].rm_so != -1 &&
-                    STRPREFIX(&token[subs[3].rm_so], "p"))
+                if (suffixstr != NULL &&
+                    STRPREFIX(suffixstr, "p"))
                     pae = 1;
                 else
                     nonpae = 1;
-            } else if (STRPREFIX(&token[subs[2].rm_so], "x86_64")) {
+            } else if (STRPREFIX(archstr, "x86_64")) {
                 arch = VIR_ARCH_X86_64;
-            } else if (STRPREFIX(&token[subs[2].rm_so], "ia64")) {
+            } else if (STRPREFIX(archstr, "ia64")) {
                 arch = VIR_ARCH_ITANIUM;
-                if (subs[3].rm_so != -1 &&
-                    STRPREFIX(&token[subs[3].rm_so], "be"))
+                if (suffixstr != NULL &&
+                    STRPREFIX(suffixstr, "be"))
                     ia64_be = 1;
-            } else if (STRPREFIX(&token[subs[2].rm_so], "powerpc64")) {
+            } else if (STRPREFIX(archstr, "powerpc64")) {
                 arch = VIR_ARCH_PPC64;
-            } else if (STRPREFIX(&token[subs[2].rm_so], "armv7l")) {
+            } else if (STRPREFIX(archstr, "armv7l")) {
                 arch = VIR_ARCH_ARMV7L;
-            } else if (STRPREFIX(&token[subs[2].rm_so], "aarch64")) {
+            } else if (STRPREFIX(archstr, "aarch64")) {
                 arch = VIR_ARCH_AARCH64;
             } else {
                 continue;
@@ -515,7 +514,6 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps)
 #endif
         }
     }
-    regfree(&regex);
 
     for (i = 0; i < nr_guest_archs; ++i) {
         virCapsGuestPtr guest;
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 08/11] src: convert over to use GRegex for regular exprssions
Posted by Pavel Hrdina 6 years, 4 months ago
On Fri, Sep 27, 2019 at 06:17:30PM +0100, Daniel P. Berrangé wrote:
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  src/conf/domain_event.c        | 25 ++++++++-----------
>  src/libxl/libxl_capabilities.c | 44 ++++++++++++++++------------------
>  2 files changed, 31 insertions(+), 38 deletions(-)

[...]

> diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c
> index 73ae0b3fa1..90d9bac9c7 100644
> --- a/src/libxl/libxl_capabilities.c
> +++ b/src/libxl/libxl_capabilities.c
> @@ -21,7 +21,6 @@
>  #include <config.h>
>  
>  #include <libxl.h>
> -#include <regex.h>
>  
>  #include "internal.h"
>  #include "virlog.h"
> @@ -374,10 +373,10 @@ static int
>  libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps)
>  {
>      const libxl_version_info *ver_info;
> -    int err;
> -    regex_t regex;
> +    g_autoptr(GRegex) regex = NULL;
> +    g_autoptr(GError) err = NULL;
> +    g_autoptr(GMatchInfo) info = NULL;
>      char *str, *token;
> -    regmatch_t subs[4];
>      char *saveptr = NULL;
>      size_t i;
>  
> @@ -398,12 +397,10 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps)
>          return -1;
>      }
>  
> -    err = regcomp(&regex, XEN_CAP_REGEX, REG_EXTENDED);
> -    if (err != 0) {
> -        char error[100];
> -        regerror(err, &regex, error, sizeof(error));
> +    regex = g_regex_new(XEN_CAP_REGEX, G_REGEX_EXTENDED, 0, &err);
> +    if (!regex) {
>          virReportError(VIR_ERR_INTERNAL_ERROR,
> -                       _("Failed to compile regex %s"), error);
> +                       _("Failed to compile regex %s"), err->message);
>          return -1;
>      }
>  
> @@ -436,31 +433,33 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps)
>           nr_guest_archs < sizeof(guest_archs) / sizeof(guest_archs[0])
>                   && (token = strtok_r(str, " ", &saveptr)) != NULL;
>           str = NULL) {
> -        if (regexec(&regex, token, sizeof(subs) / sizeof(subs[0]),
> -                    subs, 0) == 0) {
> -            int hvm = STRPREFIX(&token[subs[1].rm_so], "hvm");
> +        if (g_regex_match(regex, token, 0, &info)) {
> +            g_autofree char *modestr = g_match_info_fetch(info, 1);
> +            g_autofree char *archstr = g_match_info_fetch(info, 2);
> +            g_autofree char *suffixstr = g_match_info_fetch(info, 3);
> +            int hvm = STRPREFIX(modestr, "hvm");
>              virArch arch;
>              int pae = 0, nonpae = 0, ia64_be = 0;
>  
> -            if (STRPREFIX(&token[subs[2].rm_so], "x86_32")) {
> +            if (STRPREFIX(archstr, "x86_32")) {
>                  arch = VIR_ARCH_I686;
> -                if (subs[3].rm_so != -1 &&
> -                    STRPREFIX(&token[subs[3].rm_so], "p"))
> +                if (suffixstr != NULL &&
> +                    STRPREFIX(suffixstr, "p"))

Just a nit-pick since you are touching it, I would go for 

    if (suffixstr && STRPREFIX(suffixstr, "p"))
        pae = 1;
    else
        nonpae = 1;

To comply with our syntax style, otherwise {} would be required.

>                      pae = 1;
>                  else
>                      nonpae = 1;
> -            } else if (STRPREFIX(&token[subs[2].rm_so], "x86_64")) {
> +            } else if (STRPREFIX(archstr, "x86_64")) {
>                  arch = VIR_ARCH_X86_64;
> -            } else if (STRPREFIX(&token[subs[2].rm_so], "ia64")) {
> +            } else if (STRPREFIX(archstr, "ia64")) {
>                  arch = VIR_ARCH_ITANIUM;
> -                if (subs[3].rm_so != -1 &&
> -                    STRPREFIX(&token[subs[3].rm_so], "be"))
> +                if (suffixstr != NULL &&
> +                    STRPREFIX(suffixstr, "be"))
>                      ia64_be = 1;

Same here.

Otherwise looks good.

Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list