Re: [PATCH v3 13/14] migration: refactor: one return in forURI family functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Sep 18, 2015 at 18:05:51 +0300, 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 extend too. Anyway after such
> a heavy refactoring a little cleanup will not hurt.
> 
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@xxxxxxxxxxxxx>
> ---
>  src/libvirt-domain.c |   61 +++++++++++++++++++++++---------------------------
>  1 files changed, 28 insertions(+), 33 deletions(-)
> 
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index fc61830..f012f4b 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -4242,6 +4242,7 @@ virDomainMigrateToURI(virDomainPtr domain,
>                        const char *dname,
>                        unsigned long bandwidth)
>  {
> +    int ret = -1;
>      const char *dconnuri = NULL;
>      const char *miguri = NULL;
>  
> @@ -4252,26 +4253,24 @@ virDomainMigrateToURI(virDomainPtr domain,
>  
>      /* First checkout the source */
>      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;
>  }

Well, we tend to use cleanup labels when there's some common cleanup to
be done for both successful and error case, which is not the case here.
And by applying this patch, all *ToURI* functions will be quite
different from any other function in this file. That said, I think you
could just drop this patch :-)

Jirka

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



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]