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