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@xxxxxxxxxx>
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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list