[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[Libguestfs] [PATCH 1/2] daemon: Implement a growable strings buffer type.



From: "Richard W.M. Jones" <rjones redhat com>

Previously a lot of daemon code used three variables (a string list,
'int size' and 'int alloc') to track growable strings buffers.  This
commit implements a simple struct containing the same variables, but
using size_t instead of int:

  struct stringsbuf {
    char **argv;
    size_t size;
    size_t alloc;
  };

Use it like this:

  DECLARE_STRINGSBUF (ret);
//...
  if (add_string (&ret, str) == -1)
    return NULL;
//...
  if (end_stringsbuf (&ret) == -1)
    return NULL;
  return ret.argv;
---
 daemon/9p.c        |   23 +++++++++++------------
 daemon/available.c |    9 ++++-----
 daemon/blkid.c     |   38 ++++++++++++++++++--------------------
 daemon/daemon.h    |   33 +++++++++++++++++++++++++++------
 daemon/devsparts.c |   50 +++++++++++++++++++++++---------------------------
 daemon/ext2.c      |   17 ++++++++---------
 daemon/file.c      |   11 +++++------
 daemon/find.c      |   18 +++++++++---------
 daemon/guestfsd.c  |   51 +++++++++++++++++++++++++++------------------------
 daemon/initrd.c    |   11 +++++------
 daemon/inotify.c   |   11 +++++------
 daemon/link.c      |    9 ++++-----
 daemon/ls.c        |   15 ++++++++-------
 daemon/lvm.c       |   32 ++++++++++++++++----------------
 daemon/md.c        |   29 +++++++++++++++--------------
 daemon/mount.c     |   48 ++++++++++++++++++++++++------------------------
 16 files changed, 209 insertions(+), 196 deletions(-)

diff --git a/daemon/9p.c b/daemon/9p.c
index 8476c45..6243919 100644
--- a/daemon/9p.c
+++ b/daemon/9p.c
@@ -40,8 +40,7 @@ static char *read_whole_file (const char *filename);
 char **
 do_list_9p (void)
 {
-  char **r = NULL;
-  int size = 0, alloc = 0;
+  DECLARE_STRINGSBUF (r);
 
   DIR *dir;
 
@@ -55,10 +54,10 @@ do_list_9p (void)
      * the virtio driver isn't loaded.  Don't return an error
      * in this case, but return an empty list.
      */
-    if (add_string (&r, &size, &alloc, NULL) == -1)
+    if (end_stringsbuf (&r) == -1)
       return NULL;
 
-    return r;
+    return r.argv;
   }
 
   while (1) {
@@ -79,7 +78,7 @@ do_list_9p (void)
       if (mount_tag == 0)
         continue;
 
-      if (add_string (&r, &size, &alloc, mount_tag) == -1) {
+      if (add_string (&r, mount_tag) == -1) {
         free (mount_tag);
         closedir (dir);
         return NULL;
@@ -92,7 +91,7 @@ do_list_9p (void)
   /* Check readdir didn't fail */
   if (errno != 0) {
     reply_with_perror ("readdir: /sys/block");
-    free_stringslen (r, size);
+    free_stringslen (r.argv, r.size);
     closedir (dir);
     return NULL;
   }
@@ -100,19 +99,19 @@ do_list_9p (void)
   /* Close the directory handle */
   if (closedir (dir) == -1) {
     reply_with_perror ("closedir: /sys/block");
-    free_stringslen (r, size);
+    free_stringslen (r.argv, r.size);
     return NULL;
   }
 
-  /* Sort the tags.  Note that r might be NULL if there are no tags. */
-  if (r != NULL)
-    sort_strings (r, size);
+  /* Sort the tags. */
+  if (r.size > 0)
+    sort_strings (r.argv, r.size);
 
   /* NULL terminate the list */
-  if (add_string (&r, &size, &alloc, NULL) == -1)
+  if (end_stringsbuf (&r) == -1)
     return NULL;
 
-  return r;
+  return r.argv;
 }
 
 /* Read whole file into dynamically allocated array.  If there is an
diff --git a/daemon/available.c b/daemon/available.c
index 6d2aea2..97f1558 100644
--- a/daemon/available.c
+++ b/daemon/available.c
@@ -58,16 +58,15 @@ char **
 do_available_all_groups (void)
 {
   size_t i;
-  char **groups = NULL;
-  int size = 0, alloc = 0;
+  DECLARE_STRINGSBUF (groups);
 
   for (i = 0; optgroups[i].group != NULL; ++i) {
-    if (add_string (&groups, &size, &alloc, optgroups[i].group) == -1)
+    if (add_string (&groups, optgroups[i].group) == -1)
       return NULL;
   }
 
-  if (add_string (&groups, &size, &alloc, NULL) == -1)
+  if (end_stringsbuf (&groups) == -1)
     return NULL;
 
-  return groups;                /* caller frees */
+  return groups.argv;           /* caller frees */
 }
diff --git a/daemon/blkid.c b/daemon/blkid.c
index 83cf7f5..a6170dd 100644
--- a/daemon/blkid.c
+++ b/daemon/blkid.c
@@ -132,8 +132,7 @@ blkid_with_p_i_opt (const char *device)
   int r;
   char *out = NULL, *err = NULL;
   char **lines = NULL;
-  char **ret = NULL;
-  int size = 0, alloc = 0;
+  DECLARE_STRINGSBUF (ret);
 
   r = command (&out, &err, "blkid", "-c", "/dev/null",
                "-p", "-i", "-o", "export", device, NULL);
@@ -176,28 +175,28 @@ blkid_with_p_i_opt (const char *device)
       *eq = '\0'; eq++;
 
       /* Add the key/value pair to the output */
-      if (add_string (&ret, &size, &alloc, line) == -1 ||
-          add_string (&ret, &size, &alloc, eq) == -1) goto error;
+      if (add_string (&ret, line) == -1 ||
+          add_string (&ret, eq) == -1) goto error;
     } else {
       fprintf (stderr, "blkid: unexpected blkid output ignored: %s", line);
     }
   }
 
-  if (add_string (&ret, &size, &alloc, NULL) == -1) goto error;
+  if (end_stringsbuf (&ret) == -1) goto error;
 
   free (out);
   free (err);
   free_strings (lines);
 
-  return ret;
+  return ret.argv;
 
 error:
   free (out);
   free (err);
   if (lines)
     free_strings (lines);
-  if (ret)
-    free_strings (ret);
+  if (ret.argv)
+    free_strings (ret.argv);
 
   return NULL;
 }
@@ -206,33 +205,32 @@ static char **
 blkid_without_p_i_opt(const char *device)
 {
   char *s;
-  char **ret = NULL;
-  int size = 0, alloc = 0;
+  DECLARE_STRINGSBUF (ret);
 
-  if (add_string (&ret, &size, &alloc, "TYPE") == -1) goto error;
+  if (add_string (&ret, "TYPE") == -1) goto error;
   s = get_blkid_tag (device, "TYPE");
   if (s == NULL) goto error;
-  if (add_string (&ret, &size, &alloc, s) == -1)
+  if (add_string (&ret, s) == -1)
     goto error;
 
-  if (add_string (&ret, &size, &alloc, "LABEL") == -1) goto error;
+  if (add_string (&ret, "LABEL") == -1) goto error;
   s = get_blkid_tag (device, "LABEL");
   if (s == NULL) goto error;
-  if (add_string (&ret, &size, &alloc, s) == -1)
+  if (add_string (&ret, s) == -1)
     goto error;
 
-  if (add_string (&ret, &size, &alloc, "UUID") == -1) goto error;
+  if (add_string (&ret, "UUID") == -1) goto error;
   s = get_blkid_tag (device, "UUID");
   if (s == NULL) goto error;
-  if (add_string (&ret, &size, &alloc, s) == -1)
+  if (add_string (&ret, s) == -1)
     goto error;
 
-  if (add_string (&ret, &size, &alloc, NULL) == -1) goto error;
+  if (end_stringsbuf (&ret) == -1) goto error;
 
-  return ret;
+  return ret.argv;
 error:
-  if (ret)
-    free_strings (ret);
+  if (ret.argv)
+    free_strings (ret.argv);
   return NULL;
 }
 
diff --git a/daemon/daemon.h b/daemon/daemon.h
index b515fe4..b973a7f 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -47,14 +47,35 @@ extern int xwrite (int sock, const void *buf, size_t len)
 extern int xread (int sock, void *buf, size_t len)
   __attribute__((__warn_unused_result__));
 
-extern int add_string_nodup (char ***argv, int *size, int *alloc, char *str);
-extern int add_string (char ***argv, int *size, int *alloc, const char *str);
+/* Growable strings buffer. */
+struct stringsbuf {
+  char **argv;
+  size_t size;
+  size_t alloc;
+};
+#define DECLARE_STRINGSBUF(v) \
+  struct stringsbuf (v) = { .argv = NULL, .size = 0, .alloc = 0 }
+
+/* Append a string to the strings buffer.
+ *
+ * add_string_nodup: don't copy the string.
+ * add_string: copy the string.
+ * end_stringsbuf: NULL-terminate the buffer.
+ *
+ * All functions may fail.  If these functions return -1, then
+ * reply_with_* has been called, the strings have been freed and the
+ * buffer should no longer be used.
+ */
+extern int add_string_nodup (struct stringsbuf *sb, char *str);
+extern int add_string (struct stringsbuf *sb, const char *str);
+extern int end_stringsbuf (struct stringsbuf *sb);
+
 extern size_t count_strings (char *const *argv);
-extern void sort_strings (char **argv, int len);
+extern void sort_strings (char **argv, size_t len);
 extern void free_strings (char **argv);
-extern void free_stringslen (char **argv, int len);
+extern void free_stringslen (char **argv, size_t len);
 
-extern int is_power_of_2 (unsigned long v);
+extern char **split_lines (char *str);
 
 #define command(out,err,name,...) commandf((out),(err),0,(name),__VA_ARGS__)
 #define commandr(out,err,name,...) commandrf((out),(err),0,(name),__VA_ARGS__)
@@ -74,7 +95,7 @@ extern int commandvf (char **stdoutput, char **stderror, int flags,
 extern int commandrvf (char **stdoutput, char **stderror, int flags,
                        char const* const *argv);
 
-extern char **split_lines (char *str);
+extern int is_power_of_2 (unsigned long v);
 
 extern void trim (char *str);
 
diff --git a/daemon/devsparts.c b/daemon/devsparts.c
index 440739f..6fb0097 100644
--- a/daemon/devsparts.c
+++ b/daemon/devsparts.c
@@ -31,15 +31,13 @@
 #include "daemon.h"
 #include "actions.h"
 
-typedef int (*block_dev_func_t)(const char *dev,
-                                char ***r, int *size, int *alloc);
+typedef int (*block_dev_func_t) (const char *dev, struct stringsbuf *r);
 
 /* Execute a given function for each discovered block device */
-static char**
+static char **
 foreach_block_device (block_dev_func_t func)
 {
-  char **r = NULL;
-  int size = 0, alloc = 0;
+  DECLARE_STRINGSBUF (r);
 
   DIR *dir;
   int err = 0;
@@ -78,7 +76,7 @@ foreach_block_device (block_dev_func_t func)
       close (fd);
 
       /* Call the map function for this device */
-      if((*func)(d->d_name, &r, &size, &alloc) != 0) {
+      if((*func)(d->d_name, &r) != 0) {
         err = 1;
         break;
       }
@@ -88,7 +86,7 @@ foreach_block_device (block_dev_func_t func)
   /* Check readdir didn't fail */
   if(0 != errno) {
     reply_with_perror ("readdir: /sys/block");
-    free_stringslen(r, size);
+    free_stringslen (r.argv, r.size);
     closedir (dir);
     return NULL;
   }
@@ -96,37 +94,36 @@ foreach_block_device (block_dev_func_t func)
   /* Close the directory handle */
   if (closedir (dir) == -1) {
     reply_with_perror ("closedir: /sys/block");
-    free_stringslen(r, size);
+    free_stringslen (r.argv, r.size);
     return NULL;
   }
 
   /* Free the result list on error */
-  if(err) {
-    free_stringslen(r, size);
+  if (err) {
+    free_stringslen (r.argv, r.size);
     return NULL;
   }
 
-  /* Sort the devices.  Note that r might be NULL if there are no devices. */
-  if (r != NULL)
-    sort_strings (r, size);
+  /* Sort the devices. */
+  if (r.size > 0)
+    sort_strings (r.argv, r.size);
 
   /* NULL terminate the list */
-  if (add_string (&r, &size, &alloc, NULL) == -1) {
+  if (end_stringsbuf (&r) == -1) {
     return NULL;
   }
 
-  return r;
+  return r.argv;
 }
 
 /* Add a device to the list of devices */
 static int
-add_device(const char *device,
-           char ***const r, int *const size, int *const alloc)
+add_device (const char *device, struct stringsbuf *r)
 {
   char dev_path[256];
   snprintf (dev_path, sizeof dev_path, "/dev/%s", device);
 
-  if (add_string (r, size, alloc, dev_path) == -1) {
+  if (add_string (r, dev_path) == -1) {
     return -1;
   }
 
@@ -136,12 +133,11 @@ add_device(const char *device,
 char **
 do_list_devices (void)
 {
-  return foreach_block_device(add_device);
+  return foreach_block_device (add_device);
 }
 
 static int
-add_partitions(const char *device,
-               char ***const r, int *const size, int *const alloc)
+add_partitions (const char *device, struct stringsbuf *r)
 {
   char devdir[256];
 
@@ -151,7 +147,7 @@ add_partitions(const char *device,
   DIR *dir = opendir (devdir);
   if (!dir) {
     reply_with_perror ("opendir: %s", devdir);
-    free_stringslen (*r, *size);
+    free_stringslen (r->argv, r->size);
     return -1;
   }
 
@@ -165,7 +161,7 @@ add_partitions(const char *device,
       char part[256];
       snprintf (part, sizeof part, "/dev/%s", d->d_name);
 
-      if (add_string (r, size, alloc, part) == -1) {
+      if (add_string (r, part) == -1) {
         closedir (dir);
         return -1;
       }
@@ -173,9 +169,9 @@ add_partitions(const char *device,
   }
 
   /* Check if readdir failed */
-  if(0 != errno) {
+  if (0 != errno) {
       reply_with_perror ("readdir: %s", devdir);
-      free_stringslen(*r, *size);
+      free_stringslen (r->argv, r->size);
       closedir (dir);
       return -1;
   }
@@ -183,7 +179,7 @@ add_partitions(const char *device,
   /* Close the directory handle */
   if (closedir (dir) == -1) {
     reply_with_perror ("closedir: /sys/block/%s", device);
-    free_stringslen (*r, *size);
+    free_stringslen (r->argv, r->size);
     return -1;
   }
 
@@ -193,7 +189,7 @@ add_partitions(const char *device,
 char **
 do_list_partitions (void)
 {
-  return foreach_block_device(add_partitions);
+  return foreach_block_device (add_partitions);
 }
 
 char *
diff --git a/daemon/ext2.c b/daemon/ext2.c
index 09037fb..5defbc8 100644
--- a/daemon/ext2.c
+++ b/daemon/ext2.c
@@ -63,8 +63,7 @@ do_tune2fs_l (const char *device)
   int r;
   char *out, *err;
   char *p, *pend, *colon;
-  char **ret = NULL;
-  int size = 0, alloc = 0;
+  DECLARE_STRINGSBUF (ret);
 
   char prog[] = "tune2fs";
   if (e2prog (prog) == -1)
@@ -108,30 +107,30 @@ do_tune2fs_l (const char *device)
 
       do { colon++; } while (*colon && c_isspace (*colon));
 
-      if (add_string (&ret, &size, &alloc, p) == -1) {
+      if (add_string (&ret, p) == -1) {
         free (out);
         return NULL;
       }
       if (STREQ (colon, "<none>") ||
           STREQ (colon, "<not available>") ||
           STREQ (colon, "(none)")) {
-        if (add_string (&ret, &size, &alloc, "") == -1) {
+        if (add_string (&ret, "") == -1) {
           free (out);
           return NULL;
         }
       } else {
-        if (add_string (&ret, &size, &alloc, colon) == -1) {
+        if (add_string (&ret, colon) == -1) {
           free (out);
           return NULL;
         }
       }
     }
     else {
-      if (add_string (&ret, &size, &alloc, p) == -1) {
+      if (add_string (&ret, p) == -1) {
         free (out);
         return NULL;
       }
-      if (add_string (&ret, &size, &alloc, "") == -1) {
+      if (add_string (&ret, "") == -1) {
         free (out);
         return NULL;
       }
@@ -142,10 +141,10 @@ do_tune2fs_l (const char *device)
 
   free (out);
 
-  if (add_string (&ret, &size, &alloc, NULL) == -1)
+  if (end_stringsbuf (&ret) == -1)
     return NULL;
 
-  return ret;
+  return ret.argv;
 }
 
 int
diff --git a/daemon/file.c b/daemon/file.c
index 5235446..161ea31 100644
--- a/daemon/file.c
+++ b/daemon/file.c
@@ -156,8 +156,7 @@ do_cat (const char *path)
 char **
 do_read_lines (const char *path)
 {
-  char **r = NULL;
-  int size = 0, alloc = 0;
+  DECLARE_STRINGSBUF (r);
   FILE *fp;
   char *line = NULL;
   size_t len = 0;
@@ -179,7 +178,7 @@ do_read_lines (const char *path)
     else if (n >= 1 && line[n-1] == '\n')
       line[n-1] = '\0';
 
-    if (add_string (&r, &size, &alloc, line) == -1) {
+    if (add_string (&r, line) == -1) {
       free (line);
       fclose (fp);
       return NULL;
@@ -188,18 +187,18 @@ do_read_lines (const char *path)
 
   free (line);
 
-  if (add_string (&r, &size, &alloc, NULL) == -1) {
+  if (end_stringsbuf (&r) == -1) {
     fclose (fp);
     return NULL;
   }
 
   if (fclose (fp) == EOF) {
     reply_with_perror ("fclose: %s", path);
-    free_strings (r);
+    free_stringslen (r.argv, r.size);
     return NULL;
   }
 
-  return r;
+  return r.argv;
 }
 
 int
diff --git a/daemon/find.c b/daemon/find.c
index 0b8c243..014712e 100644
--- a/daemon/find.c
+++ b/daemon/find.c
@@ -55,8 +55,7 @@ do_find (const char *dir)
   int r, len, sysrootdirlen;
   char *cmd;
   FILE *fp;
-  char **res = NULL;
-  int size = 0, alloc = 0;
+  DECLARE_STRINGSBUF (ret);
   char *sysrootdir;
   char str[PATH_MAX];
 
@@ -105,28 +104,29 @@ do_find (const char *dir)
       continue;
 
     /* Remove the directory part of the path when adding it. */
-    if (add_string (&res, &size, &alloc, str + sysrootdirlen) == -1) {
+    if (add_string (&ret, str + sysrootdirlen) == -1) {
       pclose (fp);
       return NULL;
     }
   }
   if (pclose (fp) != 0) {
     reply_with_perror ("pclose");
-    free_stringslen (res, size);
+    free_stringslen (ret.argv, ret.size);
     return NULL;
   }
 
   if (r == -1) {
-    free_stringslen (res, size);
+    free_stringslen (ret.argv, ret.size);
     return NULL;
   }
 
-  if (add_string (&res, &size, &alloc, NULL) == -1)
-    return NULL;
+  if (ret.size > 0)
+    sort_strings (ret.argv, ret.size);
 
-  sort_strings (res, size-1);
+  if (end_stringsbuf (&ret) == -1)
+    return NULL;
 
-  return res;                        /* caller frees */
+  return ret.argv;              /* caller frees */
 }
 
 /* The code below assumes each path returned can fit into a protocol
diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c
index 721c169..6174074 100644
--- a/daemon/guestfsd.c
+++ b/daemon/guestfsd.c
@@ -417,46 +417,50 @@ xread (int sock, void *v_buf, size_t len)
 }
 
 int
-add_string_nodup (char ***argv, int *size, int *alloc, char *str)
+add_string_nodup (struct stringsbuf *sb, char *str)
 {
   char **new_argv;
 
-  if (*size >= *alloc) {
-    *alloc += 64;
-    new_argv = realloc (*argv, *alloc * sizeof (char *));
+  if (sb->size >= sb->alloc) {
+    sb->alloc += 64;
+    new_argv = realloc (sb->argv, sb->alloc * sizeof (char *));
     if (new_argv == NULL) {
       reply_with_perror ("realloc");
-      free_strings (*argv);
-      *argv = NULL;
+      free_stringslen (sb->argv, sb->size);
+      sb->argv = NULL;
       return -1;
     }
-    *argv = new_argv;
+    sb->argv = new_argv;
   }
 
-  (*argv)[*size] = str;
+  sb->argv[sb->size] = str;
+  sb->size++;
 
-  (*size)++;
   return 0;
 }
 
 int
-add_string (char ***argv, int *size, int *alloc, const char *str)
+add_string (struct stringsbuf *sb, const char *str)
 {
-  char *new_str;
+  char *new_str = NULL;
 
   if (str) {
     new_str = strdup (str);
     if (new_str == NULL) {
       reply_with_perror ("strdup");
-      free_strings (*argv);
-      *argv = NULL;
+      free_stringslen (sb->argv, sb->size);
+      sb->argv = NULL;
       return -1;
     }
-  } else {
-    new_str = NULL;
   }
 
-  return add_string_nodup (argv, size, alloc, new_str);
+  return add_string_nodup (sb, new_str);
+}
+
+int
+end_stringsbuf (struct stringsbuf *sb)
+{
+  return add_string_nodup (sb, NULL);
 }
 
 size_t
@@ -485,7 +489,7 @@ compare (const void *vp1, const void *vp2)
 }
 
 void
-sort_strings (char **argv, int len)
+sort_strings (char **argv, size_t len)
 {
   qsort (argv, len, sizeof (char *), compare);
 }
@@ -501,9 +505,9 @@ free_strings (char **argv)
 }
 
 void
-free_stringslen (char **argv, int len)
+free_stringslen (char **argv, size_t len)
 {
-  int i;
+  size_t i;
 
   for (i = 0; i < len; ++i)
     free (argv[i]);
@@ -946,8 +950,7 @@ commandrvf (char **stdoutput, char **stderror, int flags,
 char **
 split_lines (char *str)
 {
-  char **lines = NULL;
-  int size = 0, alloc = 0;
+  DECLARE_STRINGSBUF (lines);
   char *p, *pend;
 
   if (STREQ (str, ""))
@@ -965,7 +968,7 @@ split_lines (char *str)
       pend++;
     }
 
-    if (add_string (&lines, &size, &alloc, p) == -1) {
+    if (add_string (&lines, p) == -1) {
       return NULL;
     }
 
@@ -973,10 +976,10 @@ split_lines (char *str)
   }
 
  empty_list:
-  if (add_string (&lines, &size, &alloc, NULL) == -1)
+  if (end_stringsbuf (&lines) == -1)
     return NULL;
 
-  return lines;
+  return lines.argv;
 }
 
 /* Skip leading and trailing whitespace, updating the original string
diff --git a/daemon/initrd.c b/daemon/initrd.c
index d9c7050..c1ef2d1 100644
--- a/daemon/initrd.c
+++ b/daemon/initrd.c
@@ -36,8 +36,7 @@ do_initrd_list (const char *path)
   FILE *fp;
   char *cmd;
   char filename[PATH_MAX];
-  char **filenames = NULL;
-  int size = 0, alloc = 0;
+  DECLARE_STRINGSBUF (filenames);
   size_t len;
 
   /* "zcat /sysroot/<path> | cpio --quiet -it", but path must be quoted. */
@@ -62,24 +61,24 @@ do_initrd_list (const char *path)
     if (len > 0 && filename[len-1] == '\n')
       filename[len-1] = '\0';
 
-    if (add_string (&filenames, &size, &alloc, filename) == -1) {
+    if (add_string (&filenames, filename) == -1) {
       pclose (fp);
       return NULL;
     }
   }
 
-  if (add_string (&filenames, &size, &alloc, NULL) == -1) {
+  if (end_stringsbuf (&filenames) == -1) {
     pclose (fp);
     return NULL;
   }
 
   if (pclose (fp) != 0) {
     reply_with_perror ("pclose");
-    free_strings (filenames);
+    free_stringslen (filenames.argv, filenames.size);
     return NULL;
   }
 
-  return filenames;
+  return filenames.argv;
 }
 
 char *
diff --git a/daemon/inotify.c b/daemon/inotify.c
index 6c00fd0..ed425b8 100644
--- a/daemon/inotify.c
+++ b/daemon/inotify.c
@@ -301,8 +301,7 @@ do_inotify_read (void)
 char **
 do_inotify_files (void)
 {
-  char **ret = NULL;
-  int size = 0, alloc = 0;
+  DECLARE_STRINGSBUF (ret);
   unsigned int i;
   FILE *fp = NULL;
   guestfs_int_inotify_event_list *events;
@@ -359,23 +358,23 @@ do_inotify_files (void)
   }
 
   while (fgets (buf, sizeof buf, fp) != NULL) {
-    int len = strlen (buf);
+    size_t len = strlen (buf);
 
     if (len > 0 && buf[len-1] == '\n')
       buf[len-1] = '\0';
 
-    if (add_string (&ret, &size, &alloc, buf) == -1)
+    if (add_string (&ret, buf) == -1)
       goto error;
   }
 
   fclose (fp); /* implicitly closes fd */
   fp = NULL;
 
-  if (add_string (&ret, &size, &alloc, NULL) == -1)
+  if (end_stringsbuf (&ret) == -1)
     goto error;
 
   unlink (tempfile);
-  return ret;
+  return ret.argv;
 
  error:
   if (fp != NULL)
diff --git a/daemon/link.c b/daemon/link.c
index b27e72c..a8162a2 100644
--- a/daemon/link.c
+++ b/daemon/link.c
@@ -60,8 +60,7 @@ do_readlinklist (const char *path, char *const *names)
   ssize_t r;
   char link[PATH_MAX];
   const char *str;
-  char **ret = NULL;
-  int size = 0, alloc = 0;
+  DECLARE_STRINGSBUF (ret);
 
   CHROOT_IN;
   fd_cwd = open (path, O_RDONLY | O_DIRECTORY);
@@ -87,7 +86,7 @@ do_readlinklist (const char *path, char *const *names)
       str = link;
     } else
       str = "";
-    if (add_string (&ret, &size, &alloc, str) == -1) {
+    if (add_string (&ret, str) == -1) {
       close (fd_cwd);
       return NULL;
     }
@@ -95,10 +94,10 @@ do_readlinklist (const char *path, char *const *names)
 
   close (fd_cwd);
 
-  if (add_string (&ret, &size, &alloc, NULL) == -1)
+  if (end_stringsbuf (&ret) == -1)
     return NULL;
 
-  return ret;
+  return ret.argv;
 }
 
 static int
diff --git a/daemon/ls.c b/daemon/ls.c
index 1df0b6a..5c76d7e 100644
--- a/daemon/ls.c
+++ b/daemon/ls.c
@@ -32,8 +32,7 @@
 char **
 do_ls (const char *path)
 {
-  char **r = NULL;
-  int size = 0, alloc = 0;
+  DECLARE_STRINGSBUF (ret);
   DIR *dir;
   struct dirent *d;
 
@@ -50,25 +49,27 @@ do_ls (const char *path)
     if (STREQ (d->d_name, ".") || STREQ (d->d_name, ".."))
       continue;
 
-    if (add_string (&r, &size, &alloc, d->d_name) == -1) {
+    if (add_string (&ret, d->d_name) == -1) {
       closedir (dir);
       return NULL;
     }
   }
 
-  if (add_string (&r, &size, &alloc, NULL) == -1) {
+  if (ret.size > 0)
+    sort_strings (ret.argv, ret.size);
+
+  if (end_stringsbuf (&ret) == -1) {
     closedir (dir);
     return NULL;
   }
 
   if (closedir (dir) == -1) {
     reply_with_perror ("closedir: %s", path);
-    free_strings (r);
+    free_stringslen (ret.argv, ret.size);
     return NULL;
   }
 
-  sort_strings (r, size-1);
-  return r;
+  return ret.argv;
 }
 
 /* Because we can't chroot and run the ls command (since 'ls' won't
diff --git a/daemon/lvm.c b/daemon/lvm.c
index 9a71c65..8b56921 100644
--- a/daemon/lvm.c
+++ b/daemon/lvm.c
@@ -45,9 +45,8 @@ static char **
 convert_lvm_output (char *out, const char *prefix)
 {
   char *p, *pend;
-  char **r = NULL;
-  int size = 0, alloc = 0;
   int len;
+  DECLARE_STRINGSBUF (ret);
   char buf[256];
   char *str;
 
@@ -79,7 +78,7 @@ convert_lvm_output (char *out, const char *prefix)
     } else
       str = p;
 
-    if (add_string (&r, &size, &alloc, str) == -1) {
+    if (add_string (&ret, str) == -1) {
       free (out);
       return NULL;
     }
@@ -89,11 +88,13 @@ convert_lvm_output (char *out, const char *prefix)
 
   free (out);
 
-  if (add_string (&r, &size, &alloc, NULL) == -1)
+  if (ret.size > 0)
+    sort_strings (ret.argv, ret.size);
+
+  if (end_stringsbuf (&ret) == -1)
     return NULL;
 
-  sort_strings (r, size-1);
-  return r;
+  return ret.argv;
 }
 
 char **
@@ -769,8 +770,7 @@ do_lvm_canonical_lv_name (const char *device)
 char **
 do_list_dm_devices (void)
 {
-  char **ret = NULL;
-  int size = 0, alloc = 0;
+  DECLARE_STRINGSBUF (ret);
   struct dirent *d;
   DIR *dir;
   int r;
@@ -802,7 +802,7 @@ do_list_dm_devices (void)
     /* Ignore dm devices which are LVs. */
     r = lv_canonical (devname, NULL);
     if (r == -1) {
-      free_stringslen (ret, size);
+      free_stringslen (ret.argv, ret.size);
       closedir (dir);
       return NULL;
     }
@@ -810,7 +810,7 @@ do_list_dm_devices (void)
       continue;
 
     /* Not an LV, so add it. */
-    if (add_string (&ret, &size, &alloc, devname) == -1) {
+    if (add_string (&ret, devname) == -1) {
       closedir (dir);
       return NULL;
     }
@@ -819,7 +819,7 @@ do_list_dm_devices (void)
   /* Did readdir fail? */
   if (errno != 0) {
     reply_with_perror ("readdir: /dev/mapper");
-    free_stringslen (ret, size);
+    free_stringslen (ret.argv, ret.size);
     closedir (dir);
     return NULL;
   }
@@ -827,17 +827,17 @@ do_list_dm_devices (void)
   /* Close the directory handle. */
   if (closedir (dir) == -1) {
     reply_with_perror ("closedir: /dev/mapper");
-    free_stringslen (ret, size);
+    free_stringslen (ret.argv, ret.size);
     return NULL;
   }
 
   /* Sort the output (may be empty). */
-  if (ret != NULL)
-    sort_strings (ret, size);
+  if (ret.size > 0)
+    sort_strings (ret.argv, ret.size);
 
   /* NULL-terminate the list. */
-  if (add_string (&ret, &size, &alloc, NULL) == -1)
+  if (end_stringsbuf (&ret) == -1)
     return NULL;
 
-  return ret;
+  return ret.argv;
 }
diff --git a/daemon/md.c b/daemon/md.c
index 41e2c75..78e84f3 100644
--- a/daemon/md.c
+++ b/daemon/md.c
@@ -180,8 +180,7 @@ glob_errfunc (const char *epath, int eerrno)
 char **
 do_list_md_devices (void)
 {
-  char **r = NULL;
-  int size = 0, alloc = 0;
+  DECLARE_STRINGSBUF (ret);
   glob_t mds;
 
   memset(&mds, 0, sizeof(mds));
@@ -218,17 +217,19 @@ do_list_md_devices (void)
     n = mempcpy(n, &mds.gl_pathv[i][strlen(PREFIX)], len);
     *n = '\0';
 
-    if (add_string_nodup (&r, &size, &alloc, dev) == -1) goto error;
+    if (add_string_nodup (&ret, dev) == -1) goto error;
   }
 
-  if (add_string_nodup (&r, &size, &alloc, NULL) == -1) goto error;
+  if (end_stringsbuf (&ret) == -1) goto error;
   globfree (&mds);
 
-  return r;
+  return ret.argv;
 
 error:
   globfree (&mds);
-  if (r != NULL) free_strings (r);
+  if (ret.argv != NULL)
+    free_stringslen (ret.argv, ret.size);
+
   return NULL;
 }
 
@@ -241,8 +242,7 @@ do_md_detail(const char *md)
   char *out = NULL, *err = NULL;
   char **lines = NULL;
 
-  char **ret = NULL;
-  int size = 0, alloc = 0;
+  DECLARE_STRINGSBUF (ret);
 
   const char *mdadm[] = { "mdadm", "-D", "--export", md, NULL };
   r = commandv (&out, &err, mdadm);
@@ -286,8 +286,8 @@ do_md_detail(const char *md)
       }
 
       /* Add the key/value pair to the output */
-      if (add_string (&ret, &size, &alloc, line) == -1 ||
-          add_string (&ret, &size, &alloc, eq) == -1) goto error;
+      if (add_string (&ret, line) == -1 ||
+          add_string (&ret, eq) == -1) goto error;
     } else {
       /* Ignore lines with no equals sign (shouldn't happen). Log to stderr so
        * it will show up in LIBGUESTFS_DEBUG. */
@@ -299,17 +299,18 @@ do_md_detail(const char *md)
   free (err);
   free_strings (lines);
 
-  if (add_string (&ret, &size, &alloc, NULL) == -1) return NULL;
+  if (end_stringsbuf (&ret) == -1)
+    return NULL;
 
-  return ret;
+  return ret.argv;
 
 error:
   free (out);
   free (err);
   if (lines)
     free_strings (lines);
-  if (ret)
-    free_strings (ret);
+  if (ret.argv != NULL)
+    free_stringslen (ret.argv, ret.size);
 
   return NULL;
 }
diff --git a/daemon/mount.c b/daemon/mount.c
index 0c393f7..5e74ce8 100644
--- a/daemon/mount.c
+++ b/daemon/mount.c
@@ -224,8 +224,7 @@ mounts_or_mountpoints (int mp)
 {
   FILE *fp;
   struct mntent *m;
-  char **ret = NULL;
-  int size = 0, alloc = 0;
+  DECLARE_STRINGSBUF (ret);
   size_t i;
   int r;
 
@@ -241,42 +240,43 @@ mounts_or_mountpoints (int mp)
   while ((m = getmntent (fp)) != NULL) {
     /* Allow a mount directory like "/sysroot". */
     if (sysroot_len > 0 && STREQ (m->mnt_dir, sysroot)) {
-      if (add_string (&ret, &size, &alloc, m->mnt_fsname) == -1) {
+      if (add_string (&ret, m->mnt_fsname) == -1) {
       error:
         endmntent (fp);
         return NULL;
       }
       if (mp &&
-          add_string (&ret, &size, &alloc, "/") == -1)
+          add_string (&ret, "/") == -1)
         goto error;
     }
     /* Or allow a mount directory like "/sysroot/...". */
     if (STRPREFIX (m->mnt_dir, sysroot) && m->mnt_dir[sysroot_len] == '/') {
-      if (add_string (&ret, &size, &alloc, m->mnt_fsname) == -1)
+      if (add_string (&ret, m->mnt_fsname) == -1)
         goto error;
       if (mp &&
-          add_string (&ret, &size, &alloc, &m->mnt_dir[sysroot_len]) == -1)
+          add_string (&ret, &m->mnt_dir[sysroot_len]) == -1)
         goto error;
     }
   }
 
   endmntent (fp);
 
-  if (add_string (&ret, &size, &alloc, NULL) == -1)
+  if (end_stringsbuf (&ret) == -1)
     return NULL;
 
   /* Convert /dev/mapper LV paths into canonical paths (RHBZ#646432). */
-  for (i = 0; ret[i] != NULL; i += mp ? 2 : 1) {
-    if (STRPREFIX (ret[i], "/dev/mapper/") || STRPREFIX (ret[i], "/dev/dm-")) {
+  for (i = 0; ret.argv[i] != NULL; i += mp ? 2 : 1) {
+    if (STRPREFIX (ret.argv[i], "/dev/mapper/") ||
+        STRPREFIX (ret.argv[i], "/dev/dm-")) {
       char *canonical;
-      r = lv_canonical (ret[i], &canonical);
+      r = lv_canonical (ret.argv[i], &canonical);
       if (r == -1) {
-        free_strings (ret);
+        free_stringslen (ret.argv, ret.size);
         return NULL;
       }
       if (r == 1) {
-        free (ret[i]);
-        ret[i] = canonical;
+        free (ret.argv[i]);
+        ret.argv[i] = canonical;
       }
       /* Ignore the case where r == 0.  This might happen where
        * eg. a LUKS /dev/mapper device is mounted, but that won't
@@ -285,7 +285,7 @@ mounts_or_mountpoints (int mp)
     }
   }
 
-  return ret;
+  return ret.argv;
 }
 
 char **
@@ -325,8 +325,7 @@ do_umount_all (void)
 {
   FILE *fp;
   struct mntent *m;
-  char **mounts = NULL;
-  int size = 0, alloc = 0;
+  DECLARE_STRINGSBUF (mounts);
   char *err;
   int i, r;
 
@@ -342,14 +341,14 @@ do_umount_all (void)
   while ((m = getmntent (fp)) != NULL) {
     /* Allow a mount directory like "/sysroot". */
     if (sysroot_len > 0 && STREQ (m->mnt_dir, sysroot)) {
-      if (add_string (&mounts, &size, &alloc, m->mnt_dir) == -1) {
+      if (add_string (&mounts, m->mnt_dir) == -1) {
         endmntent (fp);
         return -1;
       }
     }
     /* Or allow a mount directory like "/sysroot/...". */
     if (STRPREFIX (m->mnt_dir, sysroot) && m->mnt_dir[sysroot_len] == '/') {
-      if (add_string (&mounts, &size, &alloc, m->mnt_dir) == -1) {
+      if (add_string (&mounts, m->mnt_dir) == -1) {
         endmntent (fp);
         return -1;
       }
@@ -358,21 +357,22 @@ do_umount_all (void)
 
   endmntent (fp);
 
-  qsort (mounts, size, sizeof (char *), compare_longest_first);
+  if (mounts.size > 0)
+    qsort (mounts.argv, mounts.size, sizeof (char *), compare_longest_first);
 
   /* Unmount them. */
-  for (i = 0; i < size; ++i) {
-    r = command (NULL, &err, "umount", mounts[i], NULL);
+  for (i = 0; i < mounts.size; ++i) {
+    r = command (NULL, &err, "umount", mounts.argv[i], NULL);
     if (r == -1) {
-      reply_with_error ("umount: %s: %s", mounts[i], err);
+      reply_with_error ("umount: %s: %s", mounts.argv[i], err);
       free (err);
-      free_stringslen (mounts, size);
+      free_stringslen (mounts.argv, mounts.size);
       return -1;
     }
     free (err);
   }
 
-  free_stringslen (mounts, size);
+  free_stringslen (mounts.argv, mounts.size);
 
   return 0;
 }
-- 
1.7.9.1


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]