Re: [libvirt] Dead code vs. XXX comment: remove or not?
Daniel Veillard wrote:
On Thu, Sep 03, 2009 at 06:28:13PM +0200, Jim Meyering wrote:
> clang reported that this assignment to type is a dead store,
> since type is never used after this point.
>
> This is xm_internal.c, line 1074:
>
> /* XXX Forcing to pretend its a bridge */
> if (type == -1) {
> type = 1;
> }
>
> Normally I'd just remove the whole if block, but the XXX comment
> makes me think someone has plans for this code.
>
> I'll wait for a second opinion.
Seems to me we used to expect type would be used to keep the trace
of the type of the vif, but we never use that variable, only for that
test. I would just kill it from that block
list = virConfGetValue(conf, "vif");
if (list && list->type == VIR_CONF_LIST) {
list = list->list;
while (list) {
int type = -1;
since we never use the value anywhere !
Good. Thanks for the reply.
Here's the patch.
From 7280a306ba98ea6fa3d322089c3193b114ca4da1 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Fri, 4 Sep 2009 16:49:37 +0200
Subject: [PATCH] xm_internal.c: remove dead stores of local, "type"
* src/xm_internal.c (xenXMDomainConfigParse): Remove declaration
and useless containing if-block, too.
---
src/xm_internal.c | 9 +--------
1 files changed, 1 insertions(+), 8 deletions(-)
diff --git a/src/xm_internal.c b/src/xm_internal.c
index 2595a5e..e675672 100644
--- a/src/xm_internal.c
+++ b/src/xm_internal.c
@@ -1,7 +1,7 @@
/*
* xm_internal.h: helper routines for dealing with inactive domains
*
- * Copyright (C) 2006-2007 Red Hat
+ * Copyright (C) 2006-2007, 2009 Red Hat
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -995,7 +995,6 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) {
if (list && list->type == VIR_CONF_LIST) {
list = list->list;
while (list) {
- int type = -1;
char script[PATH_MAX];
char model[10];
char ip[16];
@@ -1031,7 +1030,6 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) {
mac[len] = '\0';
} else if (STRPREFIX(key, "bridge=")) {
int len = nextkey ? (nextkey - data) : sizeof(bridge)-1;
- type = 1;
if (len > (sizeof(bridge)-1))
len = sizeof(bridge)-1;
strncpy(bridge, data, len);
@@ -1069,11 +1067,6 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) {
key = nextkey;
}
- /* XXX Forcing to pretend its a bridge */
- if (type == -1) {
- type = 1;
- }
-
if (VIR_ALLOC(net) < 0)
goto cleanup;
--
1.6.4.2.409.g85dc3