Re: [PATCH 03/11] storage: Resolve resource leaks with cmd processing

[Osier Yang <jyang@xxxxxxxxxx>]

On 2013年01月31日 03:36, John Ferlan wrote:
> ---
>   src/storage/storage_backend_disk.c | 28 +++++++++++++++-------------
>   1 file changed, 15 insertions(+), 13 deletions(-)
>
> diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c
> index 4214e97..40da306 100644
> --- a/src/storage/storage_backend_disk.c
> +++ b/src/storage/storage_backend_disk.c
> @@ -1,7 +1,7 @@
>   /*
>    * storage_backend_disk.c: storage backend for disk handling
>    *
> - * Copyright (C) 2007-2008, 2010-2012 Red Hat, Inc.
> + * Copyright (C) 2007-2008, 2010-2013 Red Hat, Inc.
>    * Copyright (C) 2007-2008 Daniel P. Berrange
>    *
>    * This library is free software; you can redistribute it and/or
> @@ -385,14 +385,7 @@ virStorageBackendDiskBuildPool(virConnectPtr conn ATTRIBUTE_UNUSED,
>   {
>       bool ok_to_mklabel = false;
>       int ret = -1;
> -    /* eg parted /dev/sda mklabel msdos */
> -    virCommandPtr cmd = virCommandNewArgList(PARTED,
> -                                             pool->def->source.devices[0].path,
> -                                             "mklabel",
> -                                             "--script",
> -                                             ((pool->def->source.format == VIR_STORAGE_POOL_DISK_DOS) ? "msdos" :
> -                                              virStoragePoolFormatDiskTypeToString(pool->def->source.format)),
> -                                             NULL);
> +    virCommandPtr cmd = NULL;
>
>       virCheckFlags(VIR_STORAGE_POOL_BUILD_OVERWRITE |
>                     VIR_STORAGE_POOL_BUILD_NO_OVERWRITE, ret);
> @@ -423,8 +416,17 @@ virStorageBackendDiskBuildPool(virConnectPtr conn ATTRIBUTE_UNUSED,
>           }
>       }
>
> -    if (ok_to_mklabel)
> +    if (ok_to_mklabel) {
> +        /* eg parted /dev/sda mklabel msdos */
> +        cmd = virCommandNewArgList(PARTED,
> +                                   pool->def->source.devices[0].path,
> +                                   "mklabel",
> +                                   "--script",
> +                                   ((pool->def->source.format == VIR_STORAGE_POOL_DISK_DOS) ? "msdos" :
> +                                   virStoragePoolFormatDiskTypeToString(pool->def->source.format)),
> +                                   NULL);
>           ret = virCommandRun(cmd, NULL);
> +    }

Good catch.

>   error:
>       virCommandFree(cmd);
> @@ -634,7 +636,7 @@ virStorageBackendDiskCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED,
>                                  virStorageVolDefPtr vol)
>   {
>       int res = -1;
> -    char *partFormat;
> +    char *partFormat = NULL;
>       unsigned long long startOffset = 0, endOffset = 0;
>       virCommandPtr cmd = virCommandNewArgList(PARTED,
>                                                pool->def->source.devices[0].path,
> @@ -646,11 +648,11 @@ virStorageBackendDiskCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED,
>           virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                          "%s", _("storage pool does not support encrypted "
>                                  "volumes"));
> -        return -1;
> +        goto cleanup;
>       }
>
>       if (virStorageBackendDiskPartFormat(pool, vol,&partFormat) != 0) {
> -        return -1;
> +        goto cleanup;
>       }
>       virCommandAddArg(cmd, partFormat);

ACK

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