[libvirt] [PATCH v2 24/32] tests: Fix memory leak in testCompareXMLToArgvFiles

[Erik Skultety]

On Fri, Feb 08, 2019 at 01:37:18PM -0500, John Ferlan wrote:
> Only one path will consume the @def; otherwise, we need to free it.
>
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  tests/storagepoolxml2argvtest.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/tests/storagepoolxml2argvtest.c b/tests/storagepoolxml2argvtest.c
> index 288b81af1d..f2a8af12b0 100644
> --- a/tests/storagepoolxml2argvtest.c
> +++ b/tests/storagepoolxml2argvtest.c
> @@ -23,6 +23,7 @@ testCompareXMLToArgvFiles(bool shouldFail,
>                            const char *cmdline)
>  {
>      int ret = -1;
> +    bool consumeDef = false;
>      virStoragePoolDefPtr def = NULL;
>      virStoragePoolObjPtr pool = NULL;
>      VIR_AUTOFREE(char *) actualCmdline = NULL;
> @@ -41,6 +42,7 @@ testCompareXMLToArgvFiles(bool shouldFail,
>              goto cleanup;
>          }
>          virStoragePoolObjSetDef(pool, def);
> +        consumeDef = true;

Long term solution should probably be that these consumers make their own copy
so that we don't need to differentiate. As for short term solution, I'd prefer
if we freed def unconditionally and thus resetting @def to NULL before issuing
break; in the _NETFS path, you'd need a def->type helper for that.

Reviewed-by: Erik Skultety <eskultet at redhat.com>

>
>          if (!(src = virStorageBackendFileSystemGetPoolSource(pool))) {
>              VIR_TEST_DEBUG("pool type %d has no pool source\n", def->type);
> @@ -83,6 +85,8 @@ testCompareXMLToArgvFiles(bool shouldFail,
>      ret = 0;
>
>   cleanup:
> +    if (!consumeDef)
> +        virStoragePoolDefFree(def);
>      virStoragePoolObjEndAPI(&pool);
>      if (shouldFail) {
>          virResetLastError();
> --
> 2.20.1
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list