Re: [PATCH v2] qemuDomainSaveInternal: fix memoryleak of virDomainDef

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

 



On 7/28/20 11:42 PM, Chuan Zheng wrote:
From: Zheng Chuan <zhengchuan@xxxxxxxxxx>

virDomainDefPtr 'def' is forgot to free after qemuDomainDefFormatLive(), fix it.

Signed-off-by: Zheng Chuan <zhengchuan@xxxxxxxxxx>
---
  src/qemu/qemu_driver.c | 9 +++------
  1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 53980d4..2dafe7c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3356,18 +3356,15 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver,
       * is NULL or whether it was the live xml of the domain moments
       * before.  */
      if (xmlin) {
-        virDomainDefPtr def = NULL;
+        g_autoptr(virDomainDef) def = NULL;
if (!(def = virDomainDefParseString(xmlin, driver->xmlopt,
                                              priv->qemuCaps,
                                              VIR_DOMAIN_DEF_PARSE_INACTIVE |
-                                            VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE))) {
+                                            VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE)))
              goto endjob;
-        }


I had actually meant the {} below this comment, not the ones above ^^. Braces are required by the libvirt coding style guide (https://libvirt.org/coding-style.html#curly-braces) if either the condition or the body is multi-lined.

-        if (!qemuDomainCheckABIStability(driver, vm, def)) {
-            virDomainDefFree(def);
+        if (!qemuDomainCheckABIStability(driver, vm, def))
              goto endjob;
-        }
          xml = qemuDomainDefFormatLive(driver, priv->qemuCaps, def, NULL, true, true);
      } else {
          xml = qemuDomainDefFormatLive(driver, priv->qemuCaps, vm->def,


This looks good now.


Reviewed-by: Laine Stump <laine@xxxxxxxxxx>


(and pushed)


Before pushing, I added back the extra braces you removed, and reworded the commit log message to be more in line with our standard template (and also noted the commit when the leak was added - 0ea479f8f6 all the way back in July 2011!)


I thought about not pushing this until after the freeze is over and 6.7.0 is released (since it is a bug, but a bug that's been in the code for 9 years), but in the end decided to push it, because 1) it's a bonafide leak in the  *non*-error path of a libvirt public API, it's very simple, and 3) I would probably forget to push it if I waited until after the freeze is over.


Thanks!




[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