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

[Eduardo Lima (Etrunko)]

On Mon, Oct 22, 2012 at 5:38 AM, Wenchao Xia <xiawenc at linux.vnet.ibm.com> wrote:
>   Oringinal implement have risk, this patch should fix it
>
> Signed-off-by: Wenchao Xia <xiawenc at linux.vnet.ibm.com>
> ---
>  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,

-- 
Eduardo de Barros Lima ◤✠◢
eblima at gmail.com