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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
> ---
>  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@xxxxxxxxxx>

>
>          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@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux