[libvirt] [PATCH v2 12/12] migration: refactor: one return in forURI family functions

[John Ferlan]

On 09/10/2015 09:20 AM, Nikolay Shirokovskiy wrote:
> May be a matter of a taste but this version with one return point in every
> function looks simplier to understand and to exetend too. Anyway after such

s/exetend/extend

> a heavy refactoring a little cleanup will not hurt.
> 
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy at virtuozzo.com>
> ---
>  src/libvirt-domain.c |   61 +++++++++++++++++++++++---------------------------
>  1 files changed, 28 insertions(+), 33 deletions(-)
> 

Although I know Daniel has ACK'd already - I figured I'd complete it too...

BTW: I can confirm the following will work in ToURI3:

const char *duri = (flags & VIR_MIGRATE_PEER2PEER) ? dconnuri : NULL;
...

Remove the dconnuri = NULL, replace 'dconnuri' with 'duri' in the
virDomainMigrateUnmanagedParams call and Coverity stops complaining.

Curiously, I see the *ToURI2 function has a similar construct without a
complaint, but it's calling virDomainMigrateUnmanaged and I'll assume
Coverity gets sufficiently boggled with the call path that it doesn't
matter. Your call if you want to change the *ToURI2 function to use a
local static...



Beyond that these changes seem perfectly reasonable to me as well.


John

> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 2d98997..aad2849 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -4216,6 +4216,7 @@ virDomainMigrateToURI(virDomainPtr domain,
>                        const char *dname,
>                        unsigned long bandwidth)
>  {
> +    int ret = -1;
>      const char *dconnuri = NULL;
>      const char *miguri = NULL;
>  
> @@ -4225,26 +4226,24 @@ virDomainMigrateToURI(virDomainPtr domain,
>      virResetLastError();
>  
>      virCheckDomainReturn(domain, -1);
> -    virCheckReadOnlyGoto(domain->conn->flags, error);
> -    virCheckNonNullArgGoto(duri, error);
> +    virCheckReadOnlyGoto(domain->conn->flags, cleanup);
> +    virCheckNonNullArgGoto(duri, cleanup);
>  
>      if (virDomainMigrateUnmanagedCheckCompat(domain, flags) < 0)
> -        goto error;
> +        goto cleanup;
>  
>      if (flags & VIR_MIGRATE_PEER2PEER)
>          dconnuri = duri;
>      else
>          miguri = duri;
>  
> -    if (virDomainMigrateUnmanaged(domain, NULL, flags,
> -                               dname, dconnuri, miguri, bandwidth) < 0)
> -        goto error;
> -
> -    return 0;
> +    ret = virDomainMigrateUnmanaged(domain, NULL, flags,
> +                                    dname, dconnuri, miguri, bandwidth);
> + cleanup:
> +    if (ret < 0)
> +        virDispatchError(domain->conn);
>  
> - error:
> -    virDispatchError(domain->conn);
> -    return -1;
> +    return ret;
>  }
>  
>  
> @@ -4343,6 +4342,7 @@ virDomainMigrateToURI2(virDomainPtr domain,
>                         const char *dname,
>                         unsigned long bandwidth)
>  {
> +    int ret = -1;
>      VIR_DOMAIN_DEBUG(domain, "dconnuri=%s, miguri=%s, dxml=%s, "
>                       "flags=%lx, dname=%s, bandwidth=%lu",
>                       NULLSTR(dconnuri), NULLSTR(miguri), NULLSTR(dxml),
> @@ -4351,23 +4351,20 @@ virDomainMigrateToURI2(virDomainPtr domain,
>      virResetLastError();
>  
>      virCheckDomainReturn(domain, -1);
> -    virCheckReadOnlyGoto(domain->conn->flags, error);
> +    virCheckReadOnlyGoto(domain->conn->flags, cleanup);
>  
>      if (virDomainMigrateUnmanagedCheckCompat(domain, flags) < 0)
> -        goto error;
> +        goto cleanup;
>  
>      if (!(flags & VIR_MIGRATE_PEER2PEER))
>          dconnuri = NULL;
>  
> -    if (virDomainMigrateUnmanaged(domain, NULL, flags,
> -                               dname, dconnuri, miguri, bandwidth) < 0)
> -        goto error;
> -
> -    return 0;
> -
> - error:
> -    virDispatchError(domain->conn);
> -    return -1;
> +    ret = virDomainMigrateUnmanaged(domain, NULL, flags,
> +                                    dname, dconnuri, miguri, bandwidth);
> + cleanup:
> +    if (ret < 0)
> +        virDispatchError(domain->conn);
> +    return ret;
>  }
>  
>  
> @@ -4415,6 +4412,7 @@ virDomainMigrateToURI3(virDomainPtr domain,
>                         unsigned int nparams,
>                         unsigned int flags)
>  {
> +    int ret = -1;
>      VIR_DOMAIN_DEBUG(domain, "dconnuri=%s, params=%p, nparms=%u flags=%x",
>                       NULLSTR(dconnuri), params, nparams, flags);
>      VIR_TYPED_PARAMS_DEBUG(params, nparams);
> @@ -4422,23 +4420,20 @@ virDomainMigrateToURI3(virDomainPtr domain,
>      virResetLastError();
>  
>      virCheckDomainReturn(domain, -1);
> -    virCheckReadOnlyGoto(domain->conn->flags, error);
> +    virCheckReadOnlyGoto(domain->conn->flags, cleanup);
>  
>      if (virDomainMigrateUnmanagedCheckCompat(domain, flags) < 0)
> -        goto error;
> +        goto cleanup;
>  
>      if (!(flags & VIR_MIGRATE_PEER2PEER))
>          dconnuri = NULL;
>  
> -    if (virDomainMigrateUnmanagedParams(domain, dconnuri,
> -                                        params, nparams, flags) < 0)
> -        goto error;
> -
> -    return 0;
> -
> - error:
> -    virDispatchError(domain->conn);
> -    return -1;
> +    ret = virDomainMigrateUnmanagedParams(domain, dconnuri,
> +                                          params, nparams, flags);
> + cleanup:
> +    if (ret < 0)
> +        virDispatchError(domain->conn);
> +    return ret;
>  }
>  
>  
>