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

Re: [libvirt] qemuDomainSnapshotLoad leak



Chris Lalancette wrote:
> On 04/07/2010 11:47 AM, Jim Meyering wrote:
>> clang reports that the assignment to "snap" below is a dead store.
>> Actually it's also a leak, and it seems like it deserves a diagnostic
>> if/when that function returns NULL.
>
> It is a dead store, that is true, but it's not a leak.  The pointer that
> is returned there is a pointer to the internal snapshot object, which
> we want to keep around.  I guess we can just do:
>
> virDomainSnapshotAssignDef(&vm->snapshots, def);
>
> And don't do the assignment at all.

Hi Chris.
Thanks for the quick feedback.

Skipping the assignment altogether sounds good.

> And yes, we should also check for
> NULL and print an error if it fails.

After poking around a little, I would prefer not to give
a diagnostic when it returns NULL, since in 2 of 3 NULL-return
paths virDomainSnapshotAssignDef already issues a diagnostic.
Doing that is also consistent with the other use of that function
in the same file.

Here's the patch:

>From ba6e3fe2c47c882a5b428b506b693c7d1ea4532a Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering redhat com>
Date: Wed, 7 Apr 2010 20:22:20 +0200
Subject: [PATCH] qemuDomainSnapshotLoad: avoid dead store

* src/qemu/qemu_driver.c (qemuDomainSnapshotLoad): Remove dead store
into "snap", as well as its declaration.
---
 src/qemu/qemu_driver.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d1684b8..f5cf1f1 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1354,7 +1354,6 @@ static void qemuDomainSnapshotLoad(void *payload,
     char *xmlStr;
     int ret;
     char *fullpath;
-    virDomainSnapshotObjPtr snap = NULL;
     virDomainSnapshotDefPtr def = NULL;
     char ebuf[1024];

@@ -1406,7 +1405,7 @@ static void qemuDomainSnapshotLoad(void *payload,
             continue;
         }

-        snap = virDomainSnapshotAssignDef(&vm->snapshots, def);
+        virDomainSnapshotAssignDef(&vm->snapshots, def);

         VIR_FREE(xmlStr);
     }
--
1.7.1.rc0.212.gbd88f


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