[Libguestfs] [PATCH v7 13/13] daemon: Link guestfsd with libutils.

Richard W.M. Jones rjones at redhat.com
Mon Jun 19 13:31:29 UTC 2017


After the previous refactoring, we are able to link the daemon to
common/utils, and also remove some of the "duplicate" functions that
the daemon carried ("duplicate" in quotes because they were often not
exact duplicates).

Also this removes the duplicate reimplementation of (most) cleanup
functions in the daemon, since those are provided by libutils now.

It also allows us in future (but not in this commit) to move utility
functions from the daemon into libutils.
---
 daemon/Makefile.am   |  8 +++++--
 daemon/augeas.c      |  2 +-
 daemon/btrfs.c       | 18 +++++++--------
 daemon/cleanups.c    | 49 +---------------------------------------
 daemon/cleanups.h    | 51 -----------------------------------------
 daemon/daemon.h      | 31 ++++++++++++++++---------
 daemon/debug.c       |  4 ++--
 daemon/echo-daemon.c |  2 +-
 daemon/guestfsd.c    | 64 ----------------------------------------------------
 daemon/ldm.c         |  2 +-
 daemon/lvm.c         |  4 ++--
 daemon/md.c          |  8 ++++---
 daemon/stat.c        |  2 +-
 docs/C_SOURCE_FILES  |  1 -
 generator/daemon.ml  |  8 +++----
 15 files changed, 53 insertions(+), 201 deletions(-)

diff --git a/daemon/Makefile.am b/daemon/Makefile.am
index 0d3dde516..db19594b8 100644
--- a/daemon/Makefile.am
+++ b/daemon/Makefile.am
@@ -49,6 +49,8 @@ endif
 guestfsd_SOURCES = \
 	../common/errnostring/errnostring.h \
 	../common/protocol/guestfs_protocol.h \
+	../common/utils/cleanups.h \
+	../common/utils/utils.h \
 	9p.c \
 	acl.c \
 	actions.h \
@@ -62,7 +64,6 @@ guestfsd_SOURCES = \
 	cap.c \
 	checksum.c \
 	cleanups.c \
-	cleanups.h \
 	cmp.c \
 	command.c \
 	command.h \
@@ -178,6 +179,7 @@ guestfsd_SOURCES = \
 guestfsd_LDADD = \
 	../common/errnostring/liberrnostring.la \
 	../common/protocol/libprotocol.la \
+	../common/utils/libutils.la \
 	$(ACL_LIBS) \
 	$(CAP_LIBS) \
 	$(YAJL_LIBS) \
@@ -206,7 +208,9 @@ guestfsd_CPPFLAGS = \
 	-I$(top_srcdir)/common/errnostring \
 	-I$(top_builddir)/common/errnostring \
 	-I$(top_srcdir)/common/protocol \
-	-I$(top_builddir)/common/protocol
+	-I$(top_builddir)/common/protocol \
+	-I$(top_srcdir)/common/utils \
+	-I$(top_builddir)/common/utils
 guestfsd_CFLAGS = \
 	$(WARN_CFLAGS) $(WERROR_CFLAGS) \
 	$(RPC_CFLAGS) \
diff --git a/daemon/augeas.c b/daemon/augeas.c
index 5adc959a5..bd54c4849 100644
--- a/daemon/augeas.c
+++ b/daemon/augeas.c
@@ -436,7 +436,7 @@ do_aug_ls (const char *path)
   if (matches == NULL)
     return NULL;		/* do_aug_match has already sent the error */
 
-  sort_strings (matches, count_strings ((void *) matches));
+  sort_strings (matches, guestfs_int_count_strings ((void *) matches));
   return matches;		/* Caller frees. */
 }
 
diff --git a/daemon/btrfs.c b/daemon/btrfs.c
index ae2310b53..5f1e5d1d0 100644
--- a/daemon/btrfs.c
+++ b/daemon/btrfs.c
@@ -152,7 +152,7 @@ do_mkfs_btrfs (char *const *devices,
                int leafsize, const char *label, const char *metadata,
                int nodesize, int sectorsize)
 {
-  const size_t nr_devices = count_strings (devices);
+  const size_t nr_devices = guestfs_int_count_strings (devices);
   const size_t MAX_ARGS = nr_devices + 64;
   const char *argv[MAX_ARGS];
   size_t i = 0, j;
@@ -500,7 +500,7 @@ do_btrfs_subvolume_list (const mountable_t *fs)
 
   guestfs_int_btrfssubvolume_list *ret = NULL;
 
-  const size_t nr_subvolumes = count_strings (lines);
+  const size_t nr_subvolumes = guestfs_int_count_strings (lines);
 
   ret = malloc (sizeof *ret);
   if (!ret) {
@@ -733,7 +733,7 @@ int
 do_btrfs_device_add (char *const *devices, const char *fs)
 {
   static int btrfs_device_add_needs_force = -1;
-  const size_t nr_devices = count_strings (devices);
+  const size_t nr_devices = guestfs_int_count_strings (devices);
   const size_t MAX_ARGS = nr_devices + 8;
   const char *argv[MAX_ARGS];
   size_t i = 0, j;
@@ -781,7 +781,7 @@ do_btrfs_device_add (char *const *devices, const char *fs)
 int
 do_btrfs_device_delete (char *const *devices, const char *fs)
 {
-  const size_t nr_devices = count_strings (devices);
+  const size_t nr_devices = guestfs_int_count_strings (devices);
 
   if (nr_devices == 0)
     return 0;
@@ -1391,7 +1391,7 @@ do_btrfs_qgroup_show (const char *path)
    *  0/5        9249849344   9249849344
    *
    */
-  const size_t nr_qgroups = count_strings (lines) - 2;
+  const size_t nr_qgroups = guestfs_int_count_strings (lines) - 2;
   guestfs_int_btrfsqgroup_list *ret = NULL;
   ret = malloc (sizeof *ret);
   if (!ret) {
@@ -1821,7 +1821,7 @@ do_btrfs_balance_status (const char *path)
   if (!lines)
     return NULL;
 
-  nlines = count_strings (lines);
+  nlines = guestfs_int_count_strings (lines);
 
   ret = calloc (1, sizeof *ret);
   if (ret == NULL) {
@@ -1938,7 +1938,7 @@ do_btrfs_scrub_status (const char *path)
   if (!lines)
     return NULL;
 
-  if (count_strings (lines) < 2) {
+  if (guestfs_int_count_strings (lines) < 2) {
     reply_with_error ("truncated output from 'btrfs scrub status -R' command");
     return NULL;
   }
@@ -2124,7 +2124,7 @@ int
 do_btrfs_image (char *const *sources, const char *image,
 		int compresslevel)
 {
-  const size_t nr_sources =  count_strings (sources);
+  const size_t nr_sources =  guestfs_int_count_strings (sources);
   const size_t MAX_ARGS = 64 + nr_sources;
   const char *argv[MAX_ARGS];
   size_t i = 0, j;
@@ -2229,7 +2229,7 @@ do_btrfs_filesystem_show (const char *device)
   if (!lines)
     return NULL;
 
-  if (count_strings (lines) < 3) {
+  if (guestfs_int_count_strings (lines) < 3) {
     reply_with_error ("truncated output from 'btrfs filesystem show' command");
     return NULL;
   }
diff --git a/daemon/cleanups.c b/daemon/cleanups.c
index 3102cf94b..b4767178a 100644
--- a/daemon/cleanups.c
+++ b/daemon/cleanups.c
@@ -24,51 +24,7 @@
 
 #include <augeas.h>
 
-#include "cleanups.h"
-
-/* Use by the CLEANUP_* macros.  Do not call these directly. */
-void
-cleanup_free (void *ptr)
-{
-  free (* (void **) ptr);
-}
-
-extern void free_strings (char **argv);
-
-void
-cleanup_free_string_list (void *ptr)
-{
-  free_strings (* (char ***) ptr);
-}
-
-void
-cleanup_unlink_free (void *ptr)
-{
-  char *filename = * (char **) ptr;
-
-  if (filename) {
-    unlink (filename);
-    free (filename);
-  }
-}
-
-void
-cleanup_close (void *ptr)
-{
-  const int fd = * (int *) ptr;
-
-  if (fd >= 0)
-    close (fd);
-}
-
-void
-cleanup_fclose (void *ptr)
-{
-  FILE *f = * (FILE **) ptr;
-
-  if (f)
-    fclose (f);
-}
+#include "daemon.h"
 
 void
 cleanup_aug_close (void *ptr)
@@ -79,9 +35,6 @@ cleanup_aug_close (void *ptr)
     aug_close (aug);
 }
 
-struct stringsbuf;
-extern void free_stringsbuf (struct stringsbuf *sb);
-
 void
 cleanup_free_stringsbuf (void *ptr)
 {
diff --git a/daemon/cleanups.h b/daemon/cleanups.h
deleted file mode 100644
index a791244cb..000000000
--- a/daemon/cleanups.h
+++ /dev/null
@@ -1,51 +0,0 @@
-/* libguestfs - the guestfsd daemon
- * Copyright (C) 2009-2015 Red Hat Inc.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, write to the Free Software Foundation, Inc.,
- * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
- */
-
-#ifndef GUESTFSD_CLEANUPS_H
-#define GUESTFSD_CLEANUPS_H
-
-/* These functions are used internally by the CLEANUP_* macros.
- * Don't call them directly.
- */
-extern void cleanup_free (void *ptr);
-extern void cleanup_free_string_list (void *ptr);
-extern void cleanup_unlink_free (void *ptr);
-extern void cleanup_close (void *ptr);
-extern void cleanup_fclose (void *ptr);
-extern void cleanup_aug_close (void *ptr);
-extern void cleanup_free_stringsbuf (void *ptr);
-
-#ifdef HAVE_ATTRIBUTE_CLEANUP
-#define CLEANUP_FREE __attribute__((cleanup(cleanup_free)))
-#define CLEANUP_FREE_STRING_LIST                        \
-    __attribute__((cleanup(cleanup_free_string_list)))
-#define CLEANUP_UNLINK_FREE __attribute__((cleanup(cleanup_unlink_free)))
-#define CLEANUP_CLOSE __attribute__((cleanup(cleanup_close)))
-#define CLEANUP_FCLOSE __attribute__((cleanup(cleanup_fclose)))
-#define CLEANUP_AUG_CLOSE __attribute__((cleanup(cleanup_aug_close)))
-#define CLEANUP_FREE_STRINGSBUF __attribute__((cleanup(cleanup_free_stringsbuf)))
-#else
-#define CLEANUP_FREE
-#define CLEANUP_FREE_STRING_LIST
-#define CLEANUP_UNLINK_FREE
-#define CLEANUP_CLOSE
-#define CLEANUP_AUG_CLOSE
-#define CLEANUP_FREE_STRINGSBUF
-#endif
-
-#endif /* GUESTFSD_CLEANUPS_H */
diff --git a/daemon/daemon.h b/daemon/daemon.h
index 400116514..be7a3bedc 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -30,9 +30,11 @@
 
 #include "guestfs_protocol.h"
 
-#include "guestfs-internal-all.h"
-
 #include "cleanups.h"
+#include "guestfs-utils.h"
+
+#include "guestfs-internal-all.h"
+
 #include "structs-cleanups.h"
 #include "command.h"
 
@@ -76,6 +78,22 @@ extern int xread (int sock, void *buf, size_t len)
 
 extern char *mountable_to_string (const mountable_t *mountable);
 
+/*-- in cleanups.c --*/
+
+/* These functions are used internally by the CLEANUP_* macros.
+ * Don't call them directly.
+ */
+extern void cleanup_aug_close (void *ptr);
+extern void cleanup_free_stringsbuf (void *ptr);
+
+#ifdef HAVE_ATTRIBUTE_CLEANUP
+#define CLEANUP_AUG_CLOSE __attribute__((cleanup(cleanup_aug_close)))
+#define CLEANUP_FREE_STRINGSBUF __attribute__((cleanup(cleanup_free_stringsbuf)))
+#else
+#define CLEANUP_AUG_CLOSE
+#define CLEANUP_FREE_STRINGSBUF
+#endif
+
 /*-- in mount.c --*/
 
 extern int mount_vfs_nochroot (const char *options, const char *vfstype,
@@ -109,21 +127,12 @@ extern int end_stringsbuf (struct stringsbuf *sb);
 extern char **take_stringsbuf (struct stringsbuf *sb);
 extern void free_stringsbuf (struct stringsbuf *sb);
 
-extern size_t count_strings (char *const *argv);
 extern void sort_strings (char **argv, size_t len);
-extern void free_strings (char **argv);
 extern void free_stringslen (char **argv, size_t len);
 
 extern void sort_device_names (char **argv, size_t len);
 extern int compare_device_names (const char *a, const char *b);
 
-/* Concatenate strings, optionally with a separator string between
- * each.  On error, these return NULL but do NOT call reply_with_* nor
- * free anything.
- */
-extern char *concat_strings (char *const *argv);
-extern char *join_strings (const char *separator, char *const *argv);
-
 extern struct stringsbuf split_lines_sb (char *str);
 extern char **split_lines (char *str);
 
diff --git a/daemon/debug.c b/daemon/debug.c
index b18d87c26..e2d43a7ca 100644
--- a/daemon/debug.c
+++ b/daemon/debug.c
@@ -444,7 +444,7 @@ debug_ldd (const char *subcmd, size_t argc, char *const *const argv)
 static char *
 debug_ls (const char *subcmd, size_t argc, char *const *const argv)
 {
-  const size_t len = count_strings (argv);
+  const size_t len = guestfs_int_count_strings (argv);
   CLEANUP_FREE const char **cargv = NULL;
   size_t i;
   int r;
@@ -477,7 +477,7 @@ debug_ls (const char *subcmd, size_t argc, char *const *const argv)
 static char *
 debug_ll (const char *subcmd, size_t argc, char *const *const argv)
 {
-  const size_t len = count_strings (argv);
+  const size_t len = guestfs_int_count_strings (argv);
   CLEANUP_FREE const char **cargv = NULL;
   size_t i;
   int r;
diff --git a/daemon/echo-daemon.c b/daemon/echo-daemon.c
index d566a9bb2..15429f072 100644
--- a/daemon/echo-daemon.c
+++ b/daemon/echo-daemon.c
@@ -28,7 +28,7 @@ do_echo_daemon (char *const *argv)
 {
   char *out;
 
-  out = join_strings (" ", argv);
+  out = guestfs_int_join_strings (" ", argv);
   if (out == NULL) {
     reply_with_perror ("malloc");
     return NULL;
diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c
index db2bb702f..b3f40628b 100644
--- a/daemon/guestfsd.c
+++ b/daemon/guestfsd.c
@@ -583,16 +583,6 @@ take_stringsbuf (struct stringsbuf *sb)
   return ret;
 }
 
-size_t
-count_strings (char *const *argv)
-{
-  size_t argc;
-
-  for (argc = 0; argv[argc] != NULL; ++argc)
-    ;
-  return argc;
-}
-
 /**
  * Returns true if C<v> is a power of 2.
  *
@@ -620,19 +610,6 @@ sort_strings (char **argv, size_t len)
 }
 
 void
-free_strings (char **argv)
-{
-  size_t argc;
-
-  if (!argv)
-    return;
-
-  for (argc = 0; argv[argc] != NULL; ++argc)
-    free (argv[argc]);
-  free (argv);
-}
-
-void
 free_stringslen (char **argv, size_t len)
 {
   size_t i;
@@ -720,47 +697,6 @@ sort_device_names (char **argv, size_t len)
   qsort (argv, len, sizeof (char *), compare_device_names_vp);
 }
 
-char *
-concat_strings (char *const *argv)
-{
-  return join_strings ("", argv);
-}
-
-char *
-join_strings (const char *separator, char *const *argv)
-{
-  size_t i, len, seplen, rlen;
-  char *r;
-
-  seplen = strlen (separator);
-
-  len = 0;
-  for (i = 0; argv[i] != NULL; ++i) {
-    if (i > 0)
-      len += seplen;
-    len += strlen (argv[i]);
-  }
-  len++; /* for final \0 */
-
-  r = malloc (len);
-  if (r == NULL)
-    return NULL;
-
-  rlen = 0;
-  for (i = 0; argv[i] != NULL; ++i) {
-    if (i > 0) {
-      memcpy (&r[rlen], separator, seplen);
-      rlen += seplen;
-    }
-    len = strlen (argv[i]);
-    memcpy (&r[rlen], argv[i], len);
-    rlen += len;
-  }
-  r[rlen] = '\0';
-
-  return r;
-}
-
 /**
  * Split an output string into a NULL-terminated list of lines,
  * wrapped into a stringsbuf.
diff --git a/daemon/ldm.c b/daemon/ldm.c
index 7753b0d82..75418e8d3 100644
--- a/daemon/ldm.c
+++ b/daemon/ldm.c
@@ -320,7 +320,7 @@ do_ldmtool_scan_devices (char * const * devices)
   int r;
   CLEANUP_FREE char *out = NULL, *err = NULL;
 
-  nr_devices = count_strings (devices);
+  nr_devices = guestfs_int_count_strings (devices);
   argv = malloc ((3 + nr_devices) * sizeof (char *));
   if (argv == NULL) {
     reply_with_perror ("malloc");
diff --git a/daemon/lvm.c b/daemon/lvm.c
index 6c57046ff..5d12b009f 100644
--- a/daemon/lvm.c
+++ b/daemon/lvm.c
@@ -337,7 +337,7 @@ do_vgcreate (const char *volgroup, char *const *physvols)
   CLEANUP_FREE char *err = NULL;
   CLEANUP_FREE const char **argv = NULL;
 
-  argc = count_strings (physvols) + 3;
+  argc = guestfs_int_count_strings (physvols) + 3;
   argv = malloc (sizeof (char *) * (argc + 1));
   if (argv == NULL) {
     reply_with_perror ("malloc");
@@ -643,7 +643,7 @@ do_vg_activate (int activate, char *const *volgroups)
   CLEANUP_FREE char *err = NULL;
   CLEANUP_FREE const char **argv = NULL;
 
-  argc = count_strings (volgroups) + 4;
+  argc = guestfs_int_count_strings (volgroups) + 4;
   argv = malloc (sizeof (char *) * (argc+1));
   if (argv == NULL) {
     reply_with_perror ("malloc");
diff --git a/daemon/md.c b/daemon/md.c
index 3f31529e2..64d98fae5 100644
--- a/daemon/md.c
+++ b/daemon/md.c
@@ -97,7 +97,8 @@ do_md_create (const char *name, char *const *devices,
     }
   }
   else
-    nrdevices = count_strings (devices) + count_bits (umissingbitmap);
+    nrdevices =
+      guestfs_int_count_strings (devices) + count_bits (umissingbitmap);
 
   if (optargs_bitmask & GUESTFS_MD_CREATE_LEVEL_BITMASK) {
     if (STRNEQ (level, "linear") && STRNEQ (level, "raid0") &&
@@ -124,10 +125,11 @@ do_md_create (const char *name, char *const *devices,
   }
 
   /* Check invariant. */
-  if (count_strings (devices) + count_bits (umissingbitmap) !=
+  if (guestfs_int_count_strings (devices) + count_bits (umissingbitmap) !=
       (size_t) (nrdevices + spare)) {
     reply_with_error ("devices (%zu) + bits set in missingbitmap (%zu) is not equal to nrdevices (%d) + spare (%d)",
-                      count_strings (devices), count_bits (umissingbitmap),
+                      guestfs_int_count_strings (devices),
+                      count_bits (umissingbitmap),
                       nrdevices, spare);
     return -1;
   }
diff --git a/daemon/stat.c b/daemon/stat.c
index 73f19226b..a1cd49245 100644
--- a/daemon/stat.c
+++ b/daemon/stat.c
@@ -127,7 +127,7 @@ do_internal_lstatnslist (const char *path, char *const *names)
   guestfs_int_statns_list *ret;
   size_t i, nr_names;
 
-  nr_names = count_strings (names);
+  nr_names = guestfs_int_count_strings (names);
 
   ret = malloc (sizeof *ret);
   if (!ret) {
diff --git a/docs/C_SOURCE_FILES b/docs/C_SOURCE_FILES
index 18671b9a1..61cdbea38 100644
--- a/docs/C_SOURCE_FILES
+++ b/docs/C_SOURCE_FILES
@@ -74,7 +74,6 @@ daemon/btrfs.c
 daemon/cap.c
 daemon/checksum.c
 daemon/cleanups.c
-daemon/cleanups.h
 daemon/cmp.c
 daemon/command.c
 daemon/command.h
diff --git a/generator/daemon.ml b/generator/daemon.ml
index 0300dc54b..2ae462864 100644
--- a/generator/daemon.ml
+++ b/generator/daemon.ml
@@ -397,7 +397,7 @@ let generate_daemon_stubs actions () =
         | RStringList (RPlainString, n)
         | RHashtable (RPlainString, RPlainString, n) ->
             pr "  struct guestfs_%s_ret ret;\n" name;
-            pr "  ret.%s.%s_len = count_strings (r);\n" n n;
+            pr "  ret.%s.%s_len = guestfs_int_count_strings (r);\n" n n;
             pr "  ret.%s.%s_val = r;\n" n n;
             pr "  reply ((xdrproc_t) &xdr_guestfs_%s_ret, (char *) &ret);\n"
               name
@@ -413,7 +413,7 @@ let generate_daemon_stubs actions () =
             pr "    free (r[i]);\n";
             pr "    r[i] = rr;\n";
             pr "  }\n";
-            pr "  ret.%s.%s_len = count_strings (r);\n" n n;
+            pr "  ret.%s.%s_len = guestfs_int_count_strings (r);\n" n n;
             pr "  ret.%s.%s_val = r;\n" n n;
             pr "  reply ((xdrproc_t) &xdr_guestfs_%s_ret, (char *) &ret);\n"
               name
@@ -428,7 +428,7 @@ let generate_daemon_stubs actions () =
             pr "    free (r[i]);\n";
             pr "    r[i] = rr;\n";
             pr "  }\n";
-            pr "  ret.%s.%s_len = count_strings (r);\n" n n;
+            pr "  ret.%s.%s_len = guestfs_int_count_strings (r);\n" n n;
             pr "  ret.%s.%s_val = r;\n" n n;
             pr "  reply ((xdrproc_t) &xdr_guestfs_%s_ret, (char *) &ret);\n"
               name
@@ -443,7 +443,7 @@ let generate_daemon_stubs actions () =
             pr "    free (r[i+1]);\n";
             pr "    r[i+1] = rr;\n";
             pr "  }\n";
-            pr "  ret.%s.%s_len = count_strings (r);\n" n n;
+            pr "  ret.%s.%s_len = guestfs_int_count_strings (r);\n" n n;
             pr "  ret.%s.%s_val = r;\n" n n;
             pr "  reply ((xdrproc_t) &xdr_guestfs_%s_ret, (char *) &ret);\n"
               name
-- 
2.13.0




More information about the Libguestfs mailing list