[Libvirt-cim] Fwd: Re: [PATCH 4/4] DevicePool, reimplement get_diskpool_config with libvirt

On Mon, Oct 22, 2012 at 5:38 AM, Wenchao Xia <xiawenc(a)linux.vnet.ibm.com&gt; wrote: > Oringinal implement have risk, this patch should fix it > > Signed-off-by: Wenchao Xia <xiawenc(a)linux.vnet.ibm.com&gt; > --- > src/Virt_DevicePool.c | 47 +++++++++++++++++++++++++++++++++++------------ > 1 files changed, 35 insertions(+), 12 deletions(-) > > diff --git a/src/Virt_DevicePool.c b/src/Virt_DevicePool.c > index 79dc108..0cb9124 100644 > --- a/src/Virt_DevicePool.c > +++ b/src/Virt_DevicePool.c > @@ -117,52 +117,75 @@ int get_disk_pool(virStoragePoolPtr poolptr, struct virt_pool **pool) > return ret; > } > > +/* This function returns the real number of pools, no negative value should be > + returned, if error happens it returns zero. */ > static int get_diskpool_config(virConnectPtr conn, > struct tmp_disk_pool **_pools) > { > - int count = 0; > + int count = 0, realcount = 0; > int i; > char ** names = NULL; > struct tmp_disk_pool *pools = NULL; > + int have_err = 0; > > count = virConnectNumOfStoragePools(conn); > - if (count <= 0) > + if (count <= 0) { > + have_err = 1; > goto out; > + } > > names = calloc(count, sizeof(char *)); > if (names == NULL) { > CU_DEBUG("Failed to alloc space for %i pool names", count); > count = 0; > + have_err = 1; > goto out; > } > > - if (virConnectListStoragePools(conn, names, count) == -1) { > + realcount = virConnectListStoragePools(conn, names, count); > + if (realcount == -1) { > CU_DEBUG("Failed to get storage pools"); > - count = 0; > + realcount = 0; > + have_err = 1; > + goto out; > + } > + if (realcount == 0) { > + CU_DEBUG("zero pools got, but prelist is %d.", count); > goto out; > } > > - pools = calloc(count, sizeof(*pools)); > + pools = calloc(realcount, sizeof(*pools)); > if (pools == NULL) { > - CU_DEBUG("Failed to alloc space for %i pool structs", count); > + CU_DEBUG("Failed to alloc space for %i pool structs", realcount); > + realcount = 0; > + have_err = 1; > goto out; > } > > - for (i = 0; i < count; i++) { > + i = 0; > + while (i < realcount) { > pools[i].tag = strdup(names[i]); > pools[i].primordial = false; > + i++; > } Any specific reason for changing the for() loop for a while() one?? > > out: > - for (i = 0; i < count; i++) > - free(names[i]); > - free(names); > + if (count > 0) { > + i = 0; > + while (i < count) { > + free(names[i]); > + i++; > + } > + free(names); > + } Same here. Best regards,