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@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; }