[Libguestfs] [PATCH] lib: Add COMPILE_REGEXP macro to hide regexp constructors/destructors.

[Pino Toscano]

On Friday 28 November 2014 14:44:30 Richard W.M. Jones wrote:
> [NOTE: this is not the complete patch.  Once ACKed, I will make the
> same mechanical change to all the other places in the library that use
> this pattern.]
> ---
>  src/guestfs-internal.h |  24 ++++++++
>  src/inspect-fs-unix.c  | 164 +++++++++++--------------------------------------
>  2 files changed, 59 insertions(+), 129 deletions(-)
> 
> diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
> index fd0c4a1..33d28f5 100644
> --- a/src/guestfs-internal.h
> +++ b/src/guestfs-internal.h
> @@ -667,6 +667,30 @@ extern int guestfs___match6 (guestfs_h *g, const char *str, const pcre *re, char
>  #define match4 guestfs___match4
>  #define match6 guestfs___match6
>  
> +/* Macro which compiles the regexp once when the library is loaded,
> + * and frees it when the library is unloaded.
> + */
> +#define COMPILE_REGEXP(name,pattern,options)                            \
> +  static void compile_regexp_##name (void) __attribute__((constructor)); \
> +  static void free_regexp_##name (void) __attribute__((destructor));    \
> +  static pcre *name;                                                    \
> +  static void                                                           \
> +  compile_regexp_##name (void)                                          \
> +  {                                                                     \
> +    const char *err;                                                    \
> +    int offset;                                                         \
> +    name = pcre_compile ((pattern), (options), &err, &offset, NULL);    \
> +    if (name == NULL) {                                                 \
> +      ignore_value (write (2, err, strlen (err)));                      \
> +      abort ();                                                         \
> +    }                                                                   \
> +  }                                                                     \
> +  static void                                                           \
> +  free_regexp_##name (void)                                             \
> +  {                                                                     \
> +    pcre_free (name);                                                   \
> +  }

Sounds okay, I guess it shouldn't introduce much overhead in the
library loading?

I also wonder whether we can delay the creation of those regexps
(i.e. not just the ones mentioned in this patch) when just needed;
something like the (untested):

#define COMPILE_REGEXP(name,pattern,options)                            \
  static void free_regexp_##name (void) __attribute__((destructor));    \
  static pcre *_internal_##name;                                        \
  static void                                                           \
  ##name (void)                                                         \
  {                                                                     \
    if (_internal_##name == NULL) {                                     \
      const char *err;                                                  \
      int offset;                                                       \
      _internal_##name = pcre_compile ((pattern), (options),            \
                                       &err, &offset, NULL);            \
      if (_internal_##name == NULL) {                                   \
        ignore_value (write (2, err, strlen (err)));                    \
        abort ();                                                       \
      }                                                                 \
    }                                                                   \
    return _internal_##name;                                            \
  }                                                                     \
  static void                                                           \
  free_regexp_##name (void)                                             \
  {                                                                     \
    if (_internal_##name)                                               \
      pcre_free (_internal_##name);                                     \
  }

using it e.g.: match (re_major_minor (), ...)

The only issue I could see is that the above is not thread-safe.

-- 
Pino Toscano