[PATCH 1/3] virsh: modify vshStringToArray to duplicate the elements too

[Peter Krempa <pkrempa@xxxxxxxxxx>]

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.
---
 tools/virsh-nodedev.c  | 18 +++++-------------
 tools/virsh-pool.c     | 10 ++++------
 tools/virsh-snapshot.c | 10 ++--------
 tools/virsh.c          |  8 +++++---
 tools/virsh.h          |  1 +
 5 files changed, 17 insertions(+), 30 deletions(-)

diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c
index 3255079..cfbf3bc 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..d6e3cb3 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -196,7 +196,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 +213,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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list