Re: [libvirt] [PATCH 15/15] util: Introduce VIR_DEFINE_AUTOPTR_FUNC for virStorageSource
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(a)redhat.com>
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