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

[Pavel Hrdina <phrdina@xxxxxxxxxx>]

On Fri, Sep 27, 2019 at 06:17:30PM +0100, Daniel P. Berrangé wrote:
> Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
> ---
>  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@xxxxxxxxxx>

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list