[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On 20/08/13 17:05, Peter Krempa wrote:
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,

Why not to destroy the use of vshStringToArray, and convert to
virStringSplit? and the comment of vshStringToArray should be updated,
since the method for free'ing the memory is different now, if we keep
using it.

Osier


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]