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

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,