10:22 a.m.
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(a)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;
}