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.

diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index 77364e899bcd7bffbda33cfe9665bcdc0c174f37..3a9975f52a265d8a8d96c4b43eb6feffb246a2bd 100644 (file)
--- 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);
-
-    if (*str == ',')
+    const char *name = NULL;
+    const char *snapshot = NULL;
+    const char *driver = NULL;
+    const char *file = NULL;
+    char **array = NULL;
+    int narray;
+    int i;
+
+    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 a5585e1012de349316e3a3c69e2d7137fe331a74..63721770e480e66018afb1ef4df43ac5feca6ab4 100644 (file)
--- 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,35 +181,46 @@ 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++;
-        }
+        else
+            nstr_tokens++;
+        str_tok++;
+    }
 
-        if (VIR_ALLOC_N(arr, nstr_tokens) < 0) {
-            virReportOOMError();
-            VIR_FREE(str_copied);
-            return -1;
-        }
+    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, ",");
-            nstr_tokens++;
-        } while (str_tok);
+    /* 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;