From 9d91a18ebf1594e76ff26d79e799763efc6b1cac Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Tue, 6 Nov 2012 17:45:09 -0700
Subject: [PATCH] virsh: make ,, escape parsing common

So far, none of the existing callers of vshStringToArray expected
the user to ever pass a literal comma; meanwhile, snapshot parsing
had rolled its own array parser.  Moving the comma escaping into
the common function won't affect any existing callers, and will make
this function reusable for adding memory handling to snapshot parsing.

As a bonus, the testsuite was already testing snapshot parsing, so
the fact that the test still passes means that we are now giving
testsuite exposure to vshStringToArray.

* tools/virsh-snapshot.c (vshParseSnapshotDiskspec): Move ,,
parsing...
* tools/virsh.c (vshStringToArray): ...into common function.
Also, vshStrdup can't fail.
---
 tools/virsh-snapshot.c | 48 ++++++++++++++++------------------
 tools/virsh.c          | 59 ++++++++++++++++++++++++++----------------
 2 files changed, 58 insertions(+), 49 deletions(-)

diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index 77364e899b..3a9975f52a 100644
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -197,33 +197,26 @@ static int
 vshParseSnapshotDiskspec(vshControl *ctl, virBufferPtr buf, const char *str)
 {
     int ret = -1;
-    char *name = NULL;
-    char *snapshot = NULL;
-    char *driver = NULL;
-    char *file = NULL;
-    char *spec = vshStrdup(ctl, str);
-    char *tmp = spec;
-    size_t len = strlen(str);
+    const char *name = NULL;
+    const char *snapshot = NULL;
+    const char *driver = NULL;
+    const char *file = NULL;
+    char **array = NULL;
+    int narray;
+    int i;
 
-    if (*str == ',')
+    narray = vshStringToArray(str, &array);
+    if (narray <= 0)
         goto cleanup;
-    name = tmp;
-    while ((tmp = strchr(tmp, ','))) {
-        if (tmp[1] == ',') {
-            /* Recognize ,, as an escape for a literal comma */
-            memmove(&tmp[1], &tmp[2], len - (tmp - spec) - 2 + 1);
-            len--;
-            tmp++;
-            continue;
-        }
-        /* Terminate previous string, look for next recognized one */
-        *tmp++ = '\0';
-        if (!snapshot && STRPREFIX(tmp, "snapshot="))
-            snapshot = tmp + strlen("snapshot=");
-        else if (!driver && STRPREFIX(tmp, "driver="))
-            driver = tmp + strlen("driver=");
-        else if (!file && STRPREFIX(tmp, "file="))
-            file = tmp + strlen("file=");
+
+    name = array[0];
+    for (i = 1; i < narray; i++) {
+        if (!snapshot && STRPREFIX(array[i], "snapshot="))
+            snapshot = array[i] + strlen("snapshot=");
+        else if (!driver && STRPREFIX(array[i], "driver="))
+            driver = array[i] + strlen("driver=");
+        else if (!file && STRPREFIX(array[i], "file="))
+            file = array[i] + strlen("file=");
         else
             goto cleanup;
     }
@@ -245,7 +238,10 @@ vshParseSnapshotDiskspec(vshControl *ctl, virBufferPtr buf, const char *str)
 cleanup:
     if (ret < 0)
         vshError(ctl, _("unable to parse diskspec: %s"), str);
-    VIR_FREE(spec);
+    if (array) {
+        VIR_FREE(*array);
+        VIR_FREE(array);
+    }
     return ret;
 }
 
diff --git a/tools/virsh.c b/tools/virsh.c
index a5585e1012..63721770e4 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -168,7 +168,9 @@ vshPrettyCapacity(unsigned long long val, const char **unit)
 
 /*
  * Convert the strings separated by ',' into array. The caller
- * must free the returned array after use.
+ * must free the first array element and the returned array after
+ * use (all other array elements belong to the memory allocated
+ * for the first array element).
  *
  * Returns the length of the filled array on success, or -1
  * on error.
@@ -179,36 +181,47 @@ vshStringToArray(const char *str,
 {
     char *str_copied = vshStrdup(NULL, str);
     char *str_tok = NULL;
+    char *tmp;
     unsigned int nstr_tokens = 0;
     char **arr = NULL;
+    size_t len = strlen(str_copied);
 
-    /* tokenize the string from user and save it's parts into an array */
-    if (str_copied) {
-        nstr_tokens = 1;
+    /* tokenize the string from user and save its parts into an array */
+    nstr_tokens = 1;
 
-        /* count the delimiters */
-        str_tok = str_copied;
-        while (*str_tok) {
-            if (*str_tok == ',')
-                nstr_tokens++;
+    /* count the delimiters, recognizing ,, as an escape for a
+     * literal comma */
+    str_tok = str_copied;
+    while ((str_tok = strchr(str_tok, ','))) {
+        if (str_tok[1] == ',')
             str_tok++;
-        }
-
-        if (VIR_ALLOC_N(arr, nstr_tokens) < 0) {
-            virReportOOMError();
-            VIR_FREE(str_copied);
-            return -1;
-        }
-
-        /* tokenize the input string */
-        nstr_tokens = 0;
-        str_tok = str_copied;
-        do {
-            arr[nstr_tokens] = strsep(&str_tok, ",");
+        else
             nstr_tokens++;
-        } while (str_tok);
+        str_tok++;
     }
 
+    if (VIR_ALLOC_N(arr, nstr_tokens) < 0) {
+        virReportOOMError();
+        VIR_FREE(str_copied);
+        return -1;
+    }
+
+    /* tokenize the input string, while treating ,, as a literal comma */
+    nstr_tokens = 0;
+    tmp = str_tok = str_copied;
+    while ((tmp = strchr(tmp, ','))) {
+        if (tmp[1] == ',') {
+            memmove(&tmp[1], &tmp[2], len - (tmp - str_copied) - 2 + 1);
+            len--;
+            tmp++;
+            continue;
+        }
+        *tmp++ = '\0';
+        arr[nstr_tokens++] = str_tok;
+        str_tok = tmp;
+    }
+    arr[nstr_tokens++] = str_tok;
+
     *array = arr;
     return nstr_tokens;
 }