On 17.09.2015 18:22, John Ferlan wrote:
>
>
> 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.
Thanx!
Now i'm going to fix all issues you found.
>
>
> 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