Refactor line splitting code in the daemon, and fix it so it works.

4 files changed, 62 insertions, 44 deletions

diff --git a/daemon/command.c b/daemon/command.c
index 1daccf6e..1a50264b 100644
--- a/daemon/command.c
+++ b/daemon/command.c
@@ -84,37 +84,16 @@ char **
 do_command_lines (char * const * const argv)
 {
   char *out;
-  char **lines = NULL;
-  int size = 0, alloc = 0;
-  char *p, *pend;
+  char **lines;
 
   out = do_command (argv);
   if (out == NULL)
     return NULL;
 
-  /* Now convert the output to a list of lines. */
-  p = out;
-  while (p) {
-    pend = strchr (p, '\n');
-    if (pend) {
-      *pend = '\0';
-      pend++;
-
-      /* Final \n?  Don't return an empty final element. */
-      if (*pend == '\0') break;
-    }
-
-    if (add_string (&lines, &size, &alloc, p) == -1) {
-      free (out);
-      return NULL;
-    }
-
-    p = pend;
-  }
-
+  lines = split_lines (out);
   free (out);
 
-  if (add_string (&lines, &size, &alloc, NULL) == -1)
+  if (lines == NULL)
     return NULL;
 
   return lines;			/* Caller frees. */
diff --git a/daemon/daemon.h b/daemon/daemon.h
index 001c7038..8ad7b7c7 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -47,6 +47,8 @@ extern int commandv (char **stdoutput, char **stderror,
 extern int commandrv (char **stdoutput, char **stderror,
 		      char * const* const argv);
 
+extern char **split_lines (char *str);
+
 extern int shell_quote (char *out, int len, const char *in);
 
 extern int verbose;
diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c
index eeb84bd1..406c1041 100644
--- a/daemon/guestfsd.c
+++ b/daemon/guestfsd.c
@@ -587,6 +587,60 @@ commandrv (char **stdoutput, char **stderror, char * const* const argv)
     return -1;
 }
 
+/* Split an output string into a NULL-terminated list of lines.
+ * Typically this is used where we have run an external command
+ * which has printed out a list of things, and we want to return
+ * an actual list.
+ *
+ * The corner cases here are quite tricky.  Note in particular:
+ *
+ *   "" -> []
+ *   "\n" -> [""]
+ *   "a\nb" -> ["a"; "b"]
+ *   "a\nb\n" -> ["a"; "b"]
+ *   "a\nb\n\n" -> ["a"; "b"; ""]
+ *
+ * The original string is written over and destroyed by this
+ * function (which is usually OK because it's the 'out' string
+ * from command()).  You can free the original string, because
+ * add_string() strdups the strings.
+ */
+char **
+split_lines (char *str)
+{
+  char **lines = NULL;
+  int size = 0, alloc = 0;
+  char *p, *pend;
+
+  if (strcmp (str, "") == 0)
+    goto empty_list;
+
+  p = str;
+  while (p) {
+    /* Empty last line? */
+    if (p[0] == '\0')
+      break;
+
+    pend = strchr (p, '\n');
+    if (pend) {
+      *pend = '\0';
+      pend++;
+    }
+
+    if (add_string (&lines, &size, &alloc, p) == -1) {
+      return NULL;
+    }
+
+    p = pend;
+  }
+
+ empty_list:
+  if (add_string (&lines, &size, &alloc, NULL) == -1)
+    return NULL;
+
+  return lines;
+}
+
 /* Quote 'in' for the shell, and write max len-1 bytes to out.  The
  * result will be NUL-terminated, even if it is truncated.
  *
diff --git a/daemon/strings.c b/daemon/strings.c
index 5e9c3a89..b26691d3 100644
--- a/daemon/strings.c
+++ b/daemon/strings.c
@@ -32,9 +32,7 @@ do_strings_e (const char *encoding, const char *path)
   char *buf;
   int r;
   char *out, *err;
-  char **lines = NULL;
-  int size = 0, alloc = 0;
-  char *p, *pend;
+  char **lines;
 
   NEED_ROOT (NULL);
   ABS_PATH (path, NULL);
@@ -60,25 +58,10 @@ do_strings_e (const char *encoding, const char *path)
   free (err);
 
   /* Now convert the output to a list of lines. */
-  p = out;
-  while (p && *p) {
-    pend = strchr (p, '\n');
-    if (pend) {
-      *pend = '\0';
-      pend++;
-    }
-
-    if (add_string (&lines, &size, &alloc, p) == -1) {
-      free (out);
-      return NULL;
-    }
-
-    p = pend;
-  }
-
+  lines = split_lines (out);
   free (out);
 
-  if (add_string (&lines, &size, &alloc, NULL) == -1)
+  if (lines == NULL)
     return NULL;
 
   return lines;			/* Caller frees. */