[libvirt] [PATCH] Fix memory leak in virDomainSnapshotDiskDefClear()

[Ján Tomko]

On 02/22/2014 08:37 PM, Nehal J Wani wrote:
> While running domainsnapshotxml2xmltest, it was found that valgrind pointed out
> the following memory leaks:
> 
> ==32176== 42 (32 direct, 10 indirect) bytes in 1 blocks are definitely lost in loss record 42 of 66
> ==32176==    at 0x4A069EE: malloc (vg_replace_malloc.c:270)
> ==32176==    by 0x4A06B62: realloc (vg_replace_malloc.c:662)
> ==32176==    by 0x4C65A07: virReallocN (viralloc.c:243)
> ==32176==    by 0x4C65B2E: virExpandN (viralloc.c:292)
> ==32176==    by 0x4C65E30: virInsertElementsN (viralloc.c:434)
> ==32176==    by 0x4CD71F3: virDomainDiskSourceDefParse (domain_conf.c:5078)
> ==32176==    by 0x4CF6EF4: virDomainSnapshotDefParseNode (snapshot_conf.c:151)
> ==32176==    by 0x4CF7314: virDomainSnapshotDefParseString (snapshot_conf.c:410)
> ==32176==    by 0x41FB8D: testCompareXMLToXMLHelper (domainsnapshotxml2xmltest.c:100)
> ==32176==    by 0x420FD1: virtTestRun (testutils.c:199)
> ==32176==    by 0x41F859: mymain (domainsnapshotxml2xmltest.c:222)
> ==32176==    by 0x42174D: virtTestMain (testutils.c:782)
> ==32176==
> ==32176== 128 (96 direct, 32 indirect) bytes in 1 blocks are definitely lost in loss record 51 of 66
> ==32176==    at 0x4A06BE0: realloc (vg_replace_malloc.c:662)
> ==32176==    by 0x4C65A07: virReallocN (viralloc.c:243)
> ==32176==    by 0x4C65B2E: virExpandN (viralloc.c:292)
> ==32176==    by 0x4C65E30: virInsertElementsN (viralloc.c:434)
> ==32176==    by 0x4CD71F3: virDomainDiskSourceDefParse (domain_conf.c:5078)
> ==32176==    by 0x4CF6EF4: virDomainSnapshotDefParseNode (snapshot_conf.c:151)
> ==32176==    by 0x4CF7314: virDomainSnapshotDefParseString (snapshot_conf.c:410)
> ==32176==    by 0x41FB8D: testCompareXMLToXMLHelper (domainsnapshotxml2xmltest.c:100)
> ==32176==    by 0x420FD1: virtTestRun (testutils.c:199)
> ==32176==    by 0x41F859: mymain (domainsnapshotxml2xmltest.c:222)
> ==32176==    by 0x42174D: virtTestMain (testutils.c:782)
> ==32176==    by 0x3E6CE1ED1C: (below main) (libc-start.c:226)
> ==32176==
> 
> ---
>  src/conf/snapshot_conf.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
> index 12b0930..475525f 100644
> --- a/src/conf/snapshot_conf.c
> +++ b/src/conf/snapshot_conf.c
> @@ -82,6 +82,7 @@ virDomainSnapshotDiskDefClear(virDomainSnapshotDiskDefPtr disk)
>  {
>      VIR_FREE(disk->name);
>      VIR_FREE(disk->file);
> +    virDomainDiskHostDefFree(disk->nhosts, disk->hosts);

This leaves nhosts and hosts at their original values, which seems to be OK
everywhere this function is called, but it might be a problem if someone tries
to reuse the disk definition instead of freeing it.

I'd rather write this as:

while (disk->nhosts)
    virDomainDiskHostDefClear(&disk->hosts[--def->nhosts])
VIR_FREE(disk->hosts)

Jan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140226/16d7e407/attachment-0001.sig>