From be34cccbd2e9ca0e756c8e40c34d01474e138c31 Mon Sep 17 00:00:00 2001
Message-Id: <be34cccbd2e9ca0e756c8e40c34d01474e138c31.1377873640.git.jdenemar@redhat.com>
From: Peter Krempa <pkrempa@redhat.com>
Date: Wed, 21 Aug 2013 12:43:30 +0200
Subject: [PATCH] virsh: modify vshStringToArray to duplicate the elements too

https://bugzilla.redhat.com/show_bug.cgi?id=999057

At a slightly larger memory expense allow stealing of items from the
string array returned from vshStringToArray and turn the result into a
string list compatible with virStringSplit. This will allow to use the
common dealloc function.

This patch also fixes a few forgotten checks of return from
vshStringToArray and one memory leak.

(cherry picked from commit d64af6ce3cce0d353238086c4e1ec0fd59217cf0)
---
 tools/virsh-nodedev.c  | 18 +++++-------------
 tools/virsh-pool.c     | 10 ++++------
 tools/virsh-snapshot.c | 10 ++--------
 tools/virsh.c          | 15 ++++++++-------
 tools/virsh.h          |  1 +
 5 files changed, 20 insertions(+), 34 deletions(-)

diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c
index 3255079..5522405 100644
--- a/tools/virsh-nodedev.c
+++ b/tools/virsh-nodedev.c
@@ -161,10 +161,7 @@ cmdNodeDeviceDestroy(vshControl *ctl, const vshCmd *cmd)
 
     ret = true;
 cleanup:
-    if (arr) {
-        VIR_FREE(*arr);
-        VIR_FREE(arr);
-    }
+    virStringFreeList(arr);
     virNodeDeviceFree(dev);
     return ret;
 }
@@ -409,7 +406,8 @@ cmdNodeListDevices(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
             vshError(ctl, "%s", _("Options --tree and --cap are incompatible"));
             return false;
         }
-        ncaps = vshStringToArray(cap_str, &caps);
+        if ((ncaps = vshStringToArray(cap_str, &caps)) < 0)
+            return false;
     }
 
     for (i = 0; i < ncaps; i++) {
@@ -503,10 +501,7 @@ cmdNodeListDevices(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
     }
 
 cleanup:
-    if (caps) {
-        VIR_FREE(*caps);
-        VIR_FREE(caps);
-    }
+    virStringFreeList(caps);
     vshNodeDeviceListFree(list);
     return ret;
 }
@@ -574,10 +569,7 @@ cmdNodeDeviceDumpXML(vshControl *ctl, const vshCmd *cmd)
 
     ret = true;
 cleanup:
-    if (arr) {
-        VIR_FREE(*arr);
-        VIR_FREE(arr);
-    }
+    virStringFreeList(arr);
     VIR_FREE(xml);
     virNodeDeviceFree(device);
     return ret;
diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
index 1622be2..b8fc8d7 100644
--- a/tools/virsh-pool.c
+++ b/tools/virsh-pool.c
@@ -995,12 +995,13 @@ cmdPoolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
         char **poolTypes = NULL;
         int npoolTypes = 0;
 
-        npoolTypes = vshStringToArray(type, &poolTypes);
+        if ((npoolTypes = vshStringToArray(type, &poolTypes)) < 0)
+            return false;
 
         for (i = 0; i < npoolTypes; i++) {
             if ((poolType = virStoragePoolTypeFromString(poolTypes[i])) < 0) {
                 vshError(ctl, "%s", _("Invalid pool type"));
-                VIR_FREE(poolTypes);
+                virStringFreeList(poolTypes);
                 return false;
             }
 
@@ -1036,10 +1037,7 @@ cmdPoolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
                 break;
             }
         }
-        if (poolTypes) {
-            VIR_FREE(*poolTypes);
-            VIR_FREE(poolTypes);
-        }
+        virStringFreeList(poolTypes);
     }
 
     if (!(list = vshStoragePoolListCollect(ctl, flags)))
diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index db9715b..e37a5b3 100644
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -261,10 +261,7 @@ vshParseSnapshotMemspec(vshControl *ctl, virBufferPtr buf, const char *str)
 cleanup:
     if (ret < 0)
         vshError(ctl, _("unable to parse memspec: %s"), str);
-    if (array) {
-        VIR_FREE(*array);
-        VIR_FREE(array);
-    }
+    virStringFreeList(array);
     return ret;
 }
 
@@ -313,10 +310,7 @@ vshParseSnapshotDiskspec(vshControl *ctl, virBufferPtr buf, const char *str)
 cleanup:
     if (ret < 0)
         vshError(ctl, _("unable to parse diskspec: %s"), str);
-    if (array) {
-        VIR_FREE(*array);
-        VIR_FREE(array);
-    }
+    virStringFreeList(array);
     return ret;
 }
 
diff --git a/tools/virsh.c b/tools/virsh.c
index f65dc79..15f529e 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -163,10 +163,9 @@ vshPrettyCapacity(unsigned long long val, const char **unit)
 }
 
 /*
- * Convert the strings separated by ',' into array. The caller
- * 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).
+ * Convert the strings separated by ',' into array. The returned
+ * array is a NULL terminated string list. The caller has to free
+ * the array using virStringFreeList or a similar method.
  *
  * Returns the length of the filled array on success, or -1
  * on error.
@@ -196,7 +195,8 @@ vshStringToArray(const char *str,
         str_tok++;
     }
 
-    if (VIR_ALLOC_N(arr, nstr_tokens) < 0) {
+    /* reserve the NULL element at the end */
+    if (VIR_ALLOC_N(arr, nstr_tokens + 1) < 0) {
         VIR_FREE(str_copied);
         return -1;
     }
@@ -212,12 +212,13 @@ vshStringToArray(const char *str,
             continue;
         }
         *tmp++ = '\0';
-        arr[nstr_tokens++] = str_tok;
+        arr[nstr_tokens++] = vshStrdup(NULL, str_tok);
         str_tok = tmp;
     }
-    arr[nstr_tokens++] = str_tok;
+    arr[nstr_tokens++] = vshStrdup(NULL, str_tok);
 
     *array = arr;
+    VIR_FREE(str_copied);
     return nstr_tokens;
 }
 
diff --git a/tools/virsh.h b/tools/virsh.h
index a407428..466ca2d 100644
--- a/tools/virsh.h
+++ b/tools/virsh.h
@@ -37,6 +37,7 @@
 # include "virerror.h"
 # include "virthread.h"
 # include "virnetdevbandwidth.h"
+# include "virstring.h"
 
 # define VSH_MAX_XML_FILE (10*1024*1024)
 
-- 
1.8.3.2