[libvirt] [PATCH v3 03/34] Adapt to VIR_STRDUP and VIR_STRNDUP in src/conf/*

[Eric Blake]

On 05/03/2013 03:22 PM, Eric Blake wrote:

>> @@ -250,7 +250,7 @@ virCapabilitiesAddHostMigrateTransport(virCapsPtr caps,
>>                       caps->host.nmigrateTrans, 1) < 0)
>>          return -1;
>>  
>> -    if ((caps->host.migrateTrans[caps->host.nmigrateTrans] = strdup(name)) == NULL)
>> +    if (VIR_STRDUP(caps->host.migrateTrans[caps->host.nmigrateTrans], name) < 0)
>>          return -1;
> 
> Same comments about changed semantics.  I've done the audit; this time:
> esx_driver.c [hmm, that one wasn't even checking for errors, but we are
> okay letting it report errors, so no change needed], qemu_capabilities.c
> [another case of going from silent to error report being good],
> xen_hypervisor.c, vmx2xmltest.c [tests are less important to fix; it
> changed from silent to reporting, which is okay], xml2vmxtest.c [another
> test].
> 
> So, of the list of five files with callers, I'd expect at least
> xen_hypervisor.c to be patched.

Correction - since esx_driver.c and qemu_capabilities are flat out
ignoring failure, they need to be patched to return failure (rather than
returning success with bad state).  But the test files can probably get
away with ignoring failure.  On the other hand, one of the easiest ways
to indicate that you have switched from silent to reporting behavior is
to mark the function as ATTRIBUTE_RETURN_CHECK, at which point, you
would then need to fix ALL callers to keep the compiler happy.

>> @@ -392,17 +392,14 @@ virCapabilitiesAddGuest(virCapsPtr caps,
>>      if (VIR_ALLOC(guest) < 0)
>>          goto no_memory;
>>  
>> -    if ((guest->ostype = strdup(ostype)) == NULL)
>> +    if (VIR_STRDUP(guest->ostype, ostype) < 0)
>>          goto no_memory;
> 
> The label is now incorrectly named.  And looking at that label:
> 
>  no_memory:
>     virCapabilitiesFreeGuest(guest);
>     return NULL;
> 
> this is another case of changing from silent to error-reporting.  Of the
> callers, I determined that at least: esx_driver.c and lxc_conf.c switch
> from silent to reporting (good, no change needed), and at least
> openvz_conf.c and phyp_driver.c now have double-OOM (fix them); I
> stopped auditing callers here.

At least here, esx_driver.c was returning errors back to its caller
(unlike the earlier case of *MigrateTransport where errors were silently
dropped), so I stand by my analysis that no change is needed to
esx_driver for this case.

Moral of the story - anywhere we change from silent to noisy, we really
do have to look at all callers on a case-by-case basis.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 621 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130503/0da777ad/attachment-0001.sig>