On Mon, Jan 18, 2010 at 04:58:37PM +0100, Jim Meyering wrote:
At first I was going to call virDomainDeviceDefFree only "if
(dev)",
but saw that it handles a NULL "dev" just fine, so it's better to skip
the test altogether, just as we do for many other free-like functions.
>From ea8511d709492f5cdc152a1eaccbccd05f036648 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Mon, 18 Jan 2010 16:55:36 +0100
Subject: [PATCH] qemu_driver: don't leak a virDomainDeviceDef buffer
* src/qemu/qemu_driver.c (qemudDomainAttachDevice): Don't leak "dev".
---
src/qemu/qemu_driver.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 365921f..1aa8af6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6062,11 +6062,11 @@ cleanup:
if (cgroup)
virCgroupFree(&cgroup);
- if (ret < 0 && dev != NULL) {
+ if (ret < 0 && dev != NULL)
if (qemuDomainSetDeviceOwnership(dom->conn, driver, dev, 1) < 0)
VIR_WARN0("Fail to restore disk device ownership");
- virDomainDeviceDefFree(dev);
- }
+ virDomainDeviceDefFree(dev);
+
if (vm)
virDomainObjUnlock(vm);
qemuDriverUnlock(driver);
--
Your fix is reasonable, but can you leave it out of GIT, because there are
a huge number of other related problems in this bit code which I've got
patches almost ready for.
Daniel
--
|: Red Hat, Engineering, London -o-
http://people.redhat.com/berrange/ :|
|:
http://libvirt.org -o-
http://virt-manager.org -o-
http://ovirt.org :|
|:
http://autobuild.org -o-
http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|