Re: [libvirt] [PATCH] remove useless code
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
...
These two are not safe to remove. Look at the whole code block in
question:
if (diskVal->list == NULL)
VIR_FREE(diskVal);
else if (virConfSetValue(conf, "disk", diskVal) < 0) {
diskVal = NULL;
goto no_memory;
}
diskVal = NULL;
In the case where virConfSetValue returned >= 0, we need to
still set diskVal = NULL.
Oops. Thanks.
However, the original code is ugly enough that I have rewritten
it to make it clear that regardless of the virConfSetValue outcome,
we don't free that variable:
if (diskVal->list != NULL) {
bool err = (virConfSetValue(conf, "disk", diskVal) < 0);
diskVal = NULL;
if (err)
goto no_memory;
}
VIR_FREE(diskVal);
Here's the adjusted part of that c-set:
From 2582d7c339e48e008f3f0066a30d3bd34cb7823e Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Wed, 4 Feb 2009 17:44:24 +0100
Subject: [PATCH] remove useless code
* src/bridge.c (brAddTap): Remove redundant errno=ENOMEM assignment
after failed strdup.
* src/cgroup.c (virCgroupFree): Remove redundant *group=NULL assignment
after VIR_FREE(*group).
* src/lxc_driver.c (lxcShutdown): Likewise.
* src/xen_unified.c (xenUnifiedClose): Likewise.
* src/xm_internal.c (xenXMDomainConfigFormat): Rewrite disk-
and net-config-setting code to be clearer.
---
src/bridge.c | 6 ++----
src/cgroup.c | 1 -
src/lxc_driver.c | 1 -
src/xen_unified.c | 1 -
src/xm_internal.c | 21 ++++++++++-----------
5 files changed, 12 insertions(+), 18 deletions(-)
diff --git a/src/bridge.c b/src/bridge.c
...
diff --git a/src/xm_internal.c b/src/xm_internal.c
index a03263c..a6e0bee 100644
--- a/src/xm_internal.c
+++ b/src/xm_internal.c
@@ -2233,14 +2233,13 @@ virConfPtr xenXMDomainConfigFormat(virConnectPtr conn,
hvm, priv->xendConfigVersion) < 0)
goto cleanup;
}
- if (diskVal->list == NULL)
- VIR_FREE(diskVal);
- else if (virConfSetValue(conf, "disk", diskVal) < 0) {
+ if (diskVal->list != NULL) {
+ bool err = (virConfSetValue(conf, "disk", diskVal) < 0);
diskVal = NULL;
- goto no_memory;
+ if (err)
+ goto no_memory;
}
- diskVal = NULL;
-
+ VIR_FREE(diskVal);
if (VIR_ALLOC(netVal) < 0)
goto no_memory;
@@ -2253,13 +2252,13 @@ virConfPtr xenXMDomainConfigFormat(virConnectPtr conn,
hvm) < 0)
goto cleanup;
}
- if (netVal->list == NULL)
- VIR_FREE(netVal);
- else if (virConfSetValue(conf, "vif", netVal) < 0) {
+ if (netVal->list != NULL) {
+ bool err = (virConfSetValue(conf, "vif", netVal) < 0);
netVal = NULL;
- goto no_memory;
+ if (err)
+ goto no_memory;
}
- netVal = NULL;
+ VIR_FREE(netVal);
if (hvm) {
if (def->nparallels) {
--
1.6.1.2.511.gc5d3f