On 2/8/19 3:07 AM, Erik Skultety wrote: > On Wed, Feb 06, 2019 at 08:41:47AM -0500, John Ferlan wrote: >> Let's make use of the auto __cleanup capabilities cleaning up any >> now unnecessary goto paths. A few methods were modified to use a >> more common methodology of defining/using @def and then stealing >> the pointer to @ret to return allowing @def to be autofree'd if >> the swap didn't occur on various return NULL; error paths. >> >> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx> > > This patch fails to compile with MinGW: > https://travis-ci.org/eskultety/libvirt/jobs/490018031 > ...although I don't understand why > Strange... Although I do have a recollection of seeing that inline error message though. Still I'll rework this one to extract out some stuff and repost as a v2 once I walk through all the changes I have made to date from code review to ensure I've covered all the comments raised. Thanks - John >> >> @@ -4977,10 +4955,9 @@ int >> virStorageFileGetBackingStoreStr(virStorageSourcePtr src, >> char **backing) >> { >> - virStorageSourcePtr tmp = NULL; >> + VIR_AUTOPTR(virStorageSource) tmp = NULL; >> VIR_AUTOFREE(char *) buf = NULL; >> ssize_t headerLen; >> - int ret = -1; >> int rv; >> >> *backing = NULL; >> @@ -5005,17 +4982,12 @@ virStorageFileGetBackingStoreStr(virStorageSourcePtr src, >> } >> >> if (!(tmp = virStorageSourceCopy(src, false))) >> - goto cleanup; >> + return -1; >> >> if (virStorageFileGetMetadataInternal(tmp, buf, headerLen, NULL) < 0) >> - goto cleanup; >> + return -1; >> >> VIR_STEAL_PTR(*backing, tmp->backingStoreRaw); >> >> - ret = 0; >> - >> - cleanup: >> - virStorageSourceFree(tmp); >> - >> - return ret; >> + return 0; >> } > > ... for example in ^this function, I don't quite understand why it thinks that > the call is unlikely, is it because virStorageFileGetBackingStoreStr is only > being called from qemuDomainSnapshotDiskDataCollect? > > Erik >