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@xxxxxxxxxxxxx>
> ---
> 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;
> }
>
>
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list