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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list