[libvirt] [PATCH] qemuDomainSnapshotCreateXML: Don't leak parsed snapshot definition

This function gets snapshot XML (provided by used) as an argument. It parses it into a local variable @def and then sets some more members (e.g. it creates a copy of live domain XML). Then it proceeds to checking if snapshot XML is valid (e.g. it contains as many disks as currently in the domain). If this fails then the control jumps to endjob label and subsequently return from the function. This is where AUTOFREE function for @def is ran. Well, because the code says to run plain VIR_FREE() we leak some memory because @def is actually an object and therefore it should have been declared as AUTOUNREF. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f01282a037..0a425b82e5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15563,7 +15563,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, virCapsPtr caps = NULL; qemuDomainObjPrivatePtr priv; virDomainSnapshotState state; - VIR_AUTOFREE(virDomainSnapshotDefPtr) def = NULL; + VIR_AUTOUNREF(virDomainSnapshotDefPtr) def = NULL; virCheckFlags(VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE | VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT | -- 2.21.0

On Tue, May 14, 2019 at 03:52:07PM +0200, Michal Privoznik wrote:
This function gets snapshot XML (provided by used) as an argument. It parses it into a local variable @def and then sets some more members (e.g. it creates a copy of live domain XML). Then it proceeds to checking if snapshot XML is valid (e.g. it contains as many disks as currently in the domain). If this fails then the control jumps to endjob label and subsequently return from the function. This is where AUTOFREE function for @def is ran. Well, because the code says to run plain VIR_FREE() we leak some memory because @def is actually an object and therefore it should have been declared as AUTOUNREF.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- Reviewed-by: Erik Skultety <eskultet@redhat.com>

On 5/14/19 8:52 AM, Michal Privoznik wrote:
This function gets snapshot XML (provided by used) as an argument. It parses it into a local variable @def and then sets some more members (e.g. it creates a copy of live domain XML). Then it proceeds to checking if snapshot XML is valid (e.g. it contains as many disks as currently in the domain). If this fails then the control jumps to endjob label and subsequently return from the function. This is where AUTOFREE function for @def is ran. Well, because the code says to run plain VIR_FREE() we leak some memory because @def is actually an object and therefore it should have been declared as AUTOUNREF.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f01282a037..0a425b82e5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15563,7 +15563,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, virCapsPtr caps = NULL; qemuDomainObjPrivatePtr priv; virDomainSnapshotState state; - VIR_AUTOFREE(virDomainSnapshotDefPtr) def = NULL; + VIR_AUTOUNREF(virDomainSnapshotDefPtr) def = NULL;
Typo'd in 57387ff5. Thanks for fixing it for me. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
participants (3)
-
Eric Blake
-
Erik Skultety
-
Michal Privoznik