[libvirt] [PATCH v2 12/12] migration: refactor: one return in forURI family functions

[Nikolay Shirokovskiy]

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 at 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.
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;
>>  }
>>  
>>  
>>