Re: [PATCH v2 15/32] storage: Use VIR_AUTOFREE for storage util

[Ján Tomko <jtomko@xxxxxxxxxx>]

On Fri, Feb 08, 2019 at 01:37:09PM -0500, John Ferlan wrote:
> Let's make use of the auto __cleanup capabilities cleaning up any
> now unnecessary goto paths.
>
> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> ---
> src/storage/storage_util.c | 335 +++++++++++++------------------------
> 1 file changed, 113 insertions(+), 222 deletions(-)
>
> @@ -1717,23 +1699,16 @@ virStorageBackendVolOpen(const char *path, struct stat *sb,
> static bool
> storageBackendIsPloopDir(char *path)
> {
> -    bool ret = false;
> -    char *root = NULL;
> -    char *desc = NULL;
> -    if (virAsprintf(&root, "%s/root.hds", path) < 0)
> -        return ret;
> -    if (!virFileExists(root))
> -        goto cleanup;
> -    if (virAsprintf(&desc, "%s/DiskDescriptor.xml", path) < 0)
> -        goto cleanup;
> -    if (!virFileExists(desc))
> -        goto cleanup;
> +    VIR_AUTOFREE(char *) root = NULL;
> +    VIR_AUTOFREE(char *) desc = NULL;
>
> -    ret = true;
> - cleanup:
> -    VIR_FREE(root);
> -    VIR_FREE(desc);
> -    return ret;
> +    if (virAsprintf(&root, "%s/root.hds", path) < 0 ||
> +        !virFileExists(root) ||
> +        virAsprintf(&desc, "%s/DiskDescriptor.xml", path) < 0 ||
> +        !virFileExists(desc))
> +        return false;

There is no need to group these conditions together.

> +
> +    return true;
> }
>
> /* In case of ploop volumes, path to volume is the path to the ploop

> @@ -4037,15 +3936,14 @@ getDeviceType(uint32_t host,
>               uint32_t lun,
>               int *type)
> {
> -    char *type_path = NULL;
>     char typestr[3];
>     char *gottype, *p;
>     FILE *typefile;
> -    int retval = 0;
> +    VIR_AUTOFREE(char *) type_path = NULL;
>
>     if (virAsprintf(&type_path, "/sys/bus/scsi/devices/%u:%u:%u:%u/type",
>                     host, bus, target, lun) < 0)
> -        goto out;
> +        return -1;

Another change of behavior, this returned 0 before.

>     typefile = fopen(type_path, "r");
>     if (typefile == NULL) {

Reviewed-by: Ján Tomko <jtomko@xxxxxxxxxx>

Jano

Attachment: signature.asc
Description: PGP signature