Richard W.M. Jones
2016-Feb-23 09:17 UTC
[Libguestfs] [PATCH 1/2] lib: Allow the COMPILE_REGEXP macro to be used everywhere.
Since the daemon links to pcre and use regular expressions, and since
the COMPILE_REGEXP macro doesn't depend on any aspects of the
library-side code (eg. the guestfs_h handle etc), we can allow the
daemon to use the COMPILE_REGEXP macro. Move the macro to
"guestfs-internal-all.h" to permit this.
---
src/guestfs-internal-all.h | 26 ++++++++++++++++++++++++++
src/guestfs-internal.h | 24 ------------------------
2 files changed, 26 insertions(+), 24 deletions(-)
diff --git a/src/guestfs-internal-all.h b/src/guestfs-internal-all.h
index 84b8fd6..af30e58 100644
--- a/src/guestfs-internal-all.h
+++ b/src/guestfs-internal-all.h
@@ -65,6 +65,32 @@
#define xdr_uint32_t xdr_u_int32_t
#endif
+/* Macro which compiles the regexp once when the program/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); \
+ }
+
/* The type field of a parsed mountable.
*
* This is used both by mountable_t in the daemon, and
diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
index 22b6c6c..3707c1b 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -710,30 +710,6 @@ extern int guestfs_int_match6 (guestfs_h *g, const char
*str, const pcre *re, ch
#define match4 guestfs_int_match4
#define match6 guestfs_int_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); \
- }
-
/* stringsbuf.c */
struct stringsbuf {
char **argv;
--
2.5.0
Richard W.M. Jones
2016-Feb-23 09:17 UTC
[Libguestfs] [PATCH 2/2] daemon: btrfs: Use COMPILE_REGEXP macro to compile regular expressions.
---
daemon/btrfs.c | 40 +++++++++++-----------------------------
1 file changed, 11 insertions(+), 29 deletions(-)
diff --git a/daemon/btrfs.c b/daemon/btrfs.c
index 2857217..3155a74 100644
--- a/daemon/btrfs.c
+++ b/daemon/btrfs.c
@@ -31,6 +31,7 @@
#include "optgroups.h"
#include "xstrtol.h"
#include "c-ctype.h"
+#include "ignore-value.h"
GUESTFSD_EXT_CMD(str_btrfs, btrfs);
GUESTFSD_EXT_CMD(str_btrfstune, btrfstune);
@@ -39,6 +40,13 @@ GUESTFSD_EXT_CMD(str_mkfs_btrfs, mkfs.btrfs);
GUESTFSD_EXT_CMD(str_umount, umount);
GUESTFSD_EXT_CMD(str_btrfsimage, btrfs-image);
+COMPILE_REGEXP (re_btrfs_subvolume_list,
+ "ID\\s+(\\d+).*\\s"
+ "top level\\s+(\\d+).*\\s"
+ "path\\s(.*)",
+ 0)
+COMPILE_REGEXP (re_btrfs_balance_status, "Balance on '.*' is
(.*)", 0)
+
int
optgroup_btrfs_available (void)
{
@@ -482,7 +490,6 @@ do_btrfs_subvolume_list (const mountable_t *fs)
*/
guestfs_int_btrfssubvolume_list *ret = NULL;
- pcre *re = NULL;
size_t nr_subvolumes = count_strings (lines);
@@ -500,17 +507,6 @@ do_btrfs_subvolume_list (const mountable_t *fs)
goto error;
}
- const char *errptr;
- int erroffset;
- re = pcre_compile ("ID\\s+(\\d+).*\\s"
- "top level\\s+(\\d+).*\\s"
- "path\\s(.*)",
- 0, &errptr, &erroffset, NULL);
- if (re == NULL) {
- reply_with_error ("pcre_compile (%i): %s", erroffset, errptr);
- goto error;
- }
-
for (i = 0; i < nr_subvolumes; ++i) {
/* To avoid allocations, reuse the 'line' buffer to store the
* path. Thus we don't need to free 'line', since it will be
@@ -520,7 +516,7 @@ do_btrfs_subvolume_list (const mountable_t *fs)
#define N_MATCHES 4
int ovector[N_MATCHES * 3];
- if (pcre_exec (re, NULL, line, strlen (line), 0, 0,
+ if (pcre_exec (re_btrfs_subvolume_list, NULL, line, strlen (line), 0, 0,
ovector, N_MATCHES * 3) < 0)
#undef N_MATCHES
{
@@ -553,8 +549,6 @@ do_btrfs_subvolume_list (const mountable_t *fs)
goto error;
}
- pcre_free (re);
-
return ret;
error:
@@ -565,9 +559,6 @@ do_btrfs_subvolume_list (const mountable_t *fs)
}
free (ret);
- if (re)
- pcre_free (re);
-
return NULL;
}
@@ -1789,11 +1780,8 @@ do_btrfs_balance_status (const char *path)
int r;
guestfs_int_btrfsbalance *ret;
size_t nlines;
- const char *errptr;
- int erroffset;
#define N_MATCH 2
int ovector[N_MATCH * 3];
- pcre *re = NULL;
path_buf = sysroot_path (path);
if (path_buf == NULL) {
@@ -1856,12 +1844,8 @@ do_btrfs_balance_status (const char *path)
return ret;
}
- re = pcre_compile ("Balance on '.*' is (.*)", 0,
&errptr, &erroffset, NULL);
- if (re == NULL) {
- reply_with_error ("pcre_compile (%i): %s", erroffset, errptr);
- goto error;
- }
- if (pcre_exec (re, NULL, lines[0], strlen (lines[0]), 0, 0,
+ if (pcre_exec (re_btrfs_balance_status,
+ NULL, lines[0], strlen (lines[0]), 0, 0,
ovector, N_MATCH * 3) < 0) {
reply_with_error ("unexpected output from 'btrfs balance
status' command: %s", lines[0]);
goto error;
@@ -1890,13 +1874,11 @@ do_btrfs_balance_status (const char *path)
goto error;
}
- pcre_free (re);
return ret;
error:
free (ret->btrfsbalance_status);
free (ret);
- pcre_free (re);
return NULL;
}
--
2.5.0
Pino Toscano
2016-Feb-23 10:35 UTC
Re: [Libguestfs] [PATCH 1/2] lib: Allow the COMPILE_REGEXP macro to be used everywhere.
On Tuesday 23 February 2016 09:17:42 Richard W.M. Jones wrote:> Since the daemon links to pcre and use regular expressions, and since > the COMPILE_REGEXP macro doesn't depend on any aspects of the > library-side code (eg. the guestfs_h handle etc), we can allow the > daemon to use the COMPILE_REGEXP macro. Move the macro to > "guestfs-internal-all.h" to permit this. > ---LGTM -- would you please move the <pcre.h> include from guestfs-internal.h to guestfs-internal-all.h as well? Thanks, -- Pino Toscano
Richard W.M. Jones
2016-Feb-23 10:41 UTC
Re: [Libguestfs] [PATCH 1/2] lib: Allow the COMPILE_REGEXP macro to be used everywhere.
On Tue, Feb 23, 2016 at 11:35:00AM +0100, Pino Toscano wrote:> On Tuesday 23 February 2016 09:17:42 Richard W.M. Jones wrote: > > Since the daemon links to pcre and use regular expressions, and since > > the COMPILE_REGEXP macro doesn't depend on any aspects of the > > library-side code (eg. the guestfs_h handle etc), we can allow the > > daemon to use the COMPILE_REGEXP macro. Move the macro to > > "guestfs-internal-all.h" to permit this. > > --- > > LGTM -- would you please move the <pcre.h> include from > guestfs-internal.h to guestfs-internal-all.h as well?Good point, although maybe it's better not to include <pcre.h> everywhere and instead just include it in the C files where it is needed? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
Possibly Parallel Threads
- [PATCH] btrfs: Fix btrfs_subvolume_list on F18
- [PATCH] lib: Add COMPILE_REGEXP macro to hide regexp constructors/destructors.
- Re: [PATCH] lib: Add COMPILE_REGEXP macro to hide regexp constructors/destructors.
- [PATCH v4 0/2] add btrfs_balance_status and btrfs_scrub_status
- [PATCH v2 0/2] add btrfs_balance_status and btrfs_scrub_status