[libvirt] [PATCH]: cleanup: remove useless if-before-VIR_FREE
Jim Meyering
jim at meyering.net
Mon Feb 2 18:31:36 UTC 2009
Jim Meyering <jim at meyering.net> wrote:
> Guido Günther <agx at sigxcpu.org> wrote:
>> attached patch removes the hardcoded port 22 when going through ssh, ssh
>> uses it by default. This makes it easier to override this via
>> .ssh/config on a per host basis instead of having to add this to the
>> connection URI explicitly.
>
> Good idea!
>
>> While at that I cleaned up some free vs. VIR_FREE usage and replaced
>> pointer intializations with 0 by NULL.
>
> Always welcome. Thanks!
>
> ACK, with one suggestion:
>
>>>From 8947ce3926829f0c30935c9a4acfc28dde9c621a Mon Sep 17 00:00:00 2001
>> From: =?utf-8?q?Guido=20G=C3=BCnther?= <agx at sigxcpu.org>
>> Date: Fri, 30 Jan 2009 20:23:29 +0100
>> Subject: [PATCH] don't hardcode ssh port to 22
> ...
>> - if (priv->hostname) {
>> - free (priv->hostname);
>> - priv->hostname = NULL;
>> - }
>> + if (priv->hostname)
>> + VIR_FREE(priv->hostname);
>
> Now that you've converted to VIR_FREE, you can also
> remove the preceding (useless) test.
>
> This made me realize our "avoid_if_before_free" syntax
> check should also be checking for uses of VIR_FREE, so
> I added that. That exposed a few more useless tests.
> The patch below corrects them:
I'm about to push the following 4 change sets.
The first was posted over the weekend:
http://thread.gmane.org/gmane.comp.emulators.libvirt/11386/focus=11627
The other three are tiny:
>From ee6dbe20c2095630721216148b69795b139fe585 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Sat, 31 Jan 2009 12:15:54 +0100
Subject: [PATCH 1/4] cleanup: remove useless if-before-VIR_FREE
* Makefile.cfg (useless_free_options): Also check for VIR_FREE.
* src/iptables.c (iptRulesFree): Remove useless if-before-VIR_FREE.
* src/remote_internal.c (remoteAuthSASL): Likewise.
* src/test.c (testOpenFromFile): Likewise.
---
Makefile.cfg | 1 +
src/iptables.c | 9 +++------
src/remote_internal.c | 4 +---
src/test.c | 11 ++++-------
4 files changed, 9 insertions(+), 16 deletions(-)
diff --git a/Makefile.cfg b/Makefile.cfg
index 44d3898..d9bccd2 100644
--- a/Makefile.cfg
+++ b/Makefile.cfg
@@ -59,6 +59,7 @@ local-checks-to-skip = \
useless_free_options = \
--name=sexpr_free \
+ --name=VIR_FREE \
--name=xmlFree \
--name=xmlXPathFreeContext \
--name=xmlXPathFreeObject
diff --git a/src/iptables.c b/src/iptables.c
index ad7fddf..c850b9e 100644
--- a/src/iptables.c
+++ b/src/iptables.c
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2007, 2008 Red Hat, Inc.
+ * Copyright (C) 2007-2009 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -325,11 +325,8 @@ iptRulesFree(iptRules *rules)
{
int i;
- if (rules->table)
- VIR_FREE(rules->table);
-
- if (rules->chain)
- VIR_FREE(rules->chain);
+ VIR_FREE(rules->table);
+ VIR_FREE(rules->chain);
if (rules->rules) {
for (i = 0; i < rules->nrules; i++)
diff --git a/src/remote_internal.c b/src/remote_internal.c
index b6e963a..a14822a 100644
--- a/src/remote_internal.c
+++ b/src/remote_internal.c
@@ -5388,9 +5388,7 @@ remoteAuthSASL (virConnectPtr conn, struct private_data *priv, int in_open,
goto cleanup;
}
- if (serverin) {
- VIR_FREE(serverin);
- }
+ VIR_FREE(serverin);
DEBUG("Client step result %d. Data %d bytes %p", err, clientoutlen, clientout);
/* Previous server call showed completion & we're now locally complete too */
diff --git a/src/test.c b/src/test.c
index 59b9370..0e0ec7c 100644
--- a/src/test.c
+++ b/src/test.c
@@ -1,7 +1,7 @@
/*
* test.c: A "mock" hypervisor for use by application unit tests
*
- * Copyright (C) 2006-2008 Red Hat, Inc.
+ * Copyright (C) 2006-2009 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -509,8 +509,7 @@ static int testOpenFromFile(virConnectPtr conn,
dom->persistent = 1;
virDomainObjUnlock(dom);
}
- if (domains != NULL)
- VIR_FREE(domains);
+ VIR_FREE(domains);
ret = virXPathNodeSet(conn, "/node/network", ctxt, &networks);
if (ret < 0) {
@@ -544,8 +543,7 @@ static int testOpenFromFile(virConnectPtr conn,
net->persistent = 1;
virNetworkObjUnlock(net);
}
- if (networks != NULL)
- VIR_FREE(networks);
+ VIR_FREE(networks);
/* Parse Storage Pool list */
ret = virXPathNodeSet(conn, "/node/pool", ctxt, &pools);
@@ -599,8 +597,7 @@ static int testOpenFromFile(virConnectPtr conn,
pool->active = 1;
virStoragePoolObjUnlock(pool);
}
- if (pools != NULL)
- VIR_FREE(pools);
+ VIR_FREE(pools);
xmlXPathFreeContext(ctxt);
xmlFreeDoc(xml);
--
1.6.1.2.443.g0d7c2
>From 228666e1e90814b46086668f9eef6783cb6cc5f0 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Sat, 31 Jan 2009 15:22:58 +0100
Subject: [PATCH 2/4] syntax-check: enable more checks
* Makefile.cfg (local-checks-to-skip): Don't skip sc_m4_quote_check.
Don't skip sc_prohibit_nonreentrant.
* Makefile.nonreentrant (NON_REENTRANT): Comment out until we've
remove all remaining uses of strerror.
---
.x-sc_m4_quote_check | 1 +
Makefile.cfg | 2 --
Makefile.nonreentrant | 2 +-
3 files changed, 2 insertions(+), 3 deletions(-)
create mode 100644 .x-sc_m4_quote_check
diff --git a/.x-sc_m4_quote_check b/.x-sc_m4_quote_check
new file mode 100644
index 0000000..10dfa97
--- /dev/null
+++ b/.x-sc_m4_quote_check
@@ -0,0 +1 @@
+^gnulib/m4/intl\.m4$
diff --git a/Makefile.cfg b/Makefile.cfg
index d9bccd2..0c2b810 100644
--- a/Makefile.cfg
+++ b/Makefile.cfg
@@ -38,13 +38,11 @@ local-checks-to-skip = \
sc_error_exit_success \
sc_file_system \
sc_immutable_NEWS \
- sc_m4_quote_check \
sc_makefile_path_separator_check \
sc_obsolete_symbols \
sc_prohibit_S_IS_definition \
sc_prohibit_atoi_atof \
sc_prohibit_jm_in_m4 \
- sc_prohibit_nonreentrant \
sc_prohibit_quote_without_use \
sc_prohibit_quotearg_without_use \
sc_prohibit_stat_st_blocks \
diff --git a/Makefile.nonreentrant b/Makefile.nonreentrant
index b567f31..13fa59d 100644
--- a/Makefile.nonreentrant
+++ b/Makefile.nonreentrant
@@ -79,7 +79,7 @@ NON_REENTRANT += setstate
NON_REENTRANT += sgetspent
NON_REENTRANT += srand48
NON_REENTRANT += srandom
-NON_REENTRANT += strerror
+# NON_REENTRANT += strerror
NON_REENTRANT += strtok
NON_REENTRANT += tmpnam
NON_REENTRANT += ttyname
--
1.6.1.2.443.g0d7c2
>From 5341defb0f5e5388670bab3ea57457fb0375d8d9 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Sat, 31 Jan 2009 15:31:09 +0100
Subject: [PATCH 3/4] build: enable redundant-const check
* Makefile.cfg (local-checks-to-skip): Remove sc_redundant_const.
* src/lxc_controller.c: Remove redundant "const"(s).
* src/storage_backend_fs.c: Likewise.
* src/util.h: Likewise.
* src/xen_internal.c: Likewise.
* tests/qparamtest.c: Likewise.
---
Makefile.cfg | 1 -
src/lxc_controller.c | 3 +--
src/storage_backend_fs.c | 6 +++---
src/util.h | 2 +-
src/xen_internal.c | 4 ++--
tests/qparamtest.c | 12 ++++++------
6 files changed, 13 insertions(+), 15 deletions(-)
diff --git a/Makefile.cfg b/Makefile.cfg
index 0c2b810..b93d8dc 100644
--- a/Makefile.cfg
+++ b/Makefile.cfg
@@ -46,7 +46,6 @@ local-checks-to-skip = \
sc_prohibit_quote_without_use \
sc_prohibit_quotearg_without_use \
sc_prohibit_stat_st_blocks \
- sc_redundant_const \
sc_root_tests \
sc_space_tab \
sc_sun_os_names \
diff --git a/src/lxc_controller.c b/src/lxc_controller.c
index e25fe76..58dfe02 100644
--- a/src/lxc_controller.c
+++ b/src/lxc_controller.c
@@ -507,7 +507,7 @@ int main(int argc, char *argv[])
virDomainDefPtr def = NULL;
char *configFile = NULL;
char *sockpath = NULL;
- const struct option const options[] = {
+ const struct option options[] = {
{ "background", 0, NULL, 'b' },
{ "name", 1, NULL, 'n' },
{ "veth", 1, NULL, 'v' },
@@ -662,4 +662,3 @@ cleanup:
return rc;
}
-
diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c
index 345dc40..240de96 100644
--- a/src/storage_backend_fs.c
+++ b/src/storage_backend_fs.c
@@ -1,7 +1,7 @@
/*
* storage_backend_fs.c: storage backend for FS and directory handling
*
- * Copyright (C) 2007-2008 Red Hat, Inc.
+ * Copyright (C) 2007-2009 Red Hat, Inc.
* Copyright (C) 2007-2008 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -84,7 +84,7 @@ struct FileTypeInfo {
int (*getBackingStore)(virConnectPtr conn, char **res,
const unsigned char *buf, size_t buf_size);
};
-const struct FileTypeInfo const fileTypeInfo[] = {
+struct FileTypeInfo const fileTypeInfo[] = {
/* Bochs */
/* XXX Untested
{ VIR_STORAGE_VOL_BOCHS, "Bochs Virtual HD Image", NULL,
@@ -1020,7 +1020,7 @@ virStorageBackendFileSystemVolCreate(virConnectPtr conn,
* Need to add in progress bars & bg thread somehow */
if (vol->allocation) {
unsigned long long remain = vol->allocation;
- static const char const zeros[4096];
+ static char const zeros[4096];
while (remain) {
int bytes = sizeof(zeros);
if (bytes > remain)
diff --git a/src/util.h b/src/util.h
index e731ba4..c553264 100644
--- a/src/util.h
+++ b/src/util.h
@@ -143,7 +143,7 @@ const char *virEnumToString(const char *const*types,
int type);
#define VIR_ENUM_IMPL(name, lastVal, ...) \
- static const char const *name ## TypeList[] = { __VA_ARGS__ }; \
+ static const char *const name ## TypeList[] = { __VA_ARGS__ }; \
extern int (* name ## Verify (void)) [verify_true (ARRAY_CARDINALITY(name ## TypeList) == lastVal)]; \
const char *name ## TypeToString(int type) { \
return virEnumToString(name ## TypeList, \
diff --git a/src/xen_internal.c b/src/xen_internal.c
index 9a7272f..0a01f5e 100644
--- a/src/xen_internal.c
+++ b/src/xen_internal.c
@@ -1,7 +1,7 @@
/*
* xen_internal.c: direct access to Xen hypervisor level
*
- * Copyright (C) 2005, 2006, 2007, 2008 Red Hat, Inc.
+ * Copyright (C) 2005, 2006, 2007, 2008, 2009 Red Hat, Inc.
*
* See COPYING.LIB for the License of this software
*
@@ -2178,7 +2178,7 @@ xenHypervisorBuildCapabilities(virConnectPtr conn,
for (i = 0; i < nr_guest_archs; ++i) {
virCapsGuestPtr guest;
- const char const *machines[] = { guest_archs[i].hvm ? "xenfv" : "xenpv" };
+ char const *const machines[] = {guest_archs[i].hvm ? "xenfv" : "xenpv"};
if ((guest = virCapabilitiesAddGuest(caps,
guest_archs[i].hvm ? "hvm" : "xen",
diff --git a/tests/qparamtest.c b/tests/qparamtest.c
index f8f2d29..a4ed1fb 100644
--- a/tests/qparamtest.c
+++ b/tests/qparamtest.c
@@ -175,12 +175,12 @@ fail:
return ret;
}
-static const struct qparamParseDataEntry const params1[] = { { "foo", "one" }, { "bar", "two" } };
-static const struct qparamParseDataEntry const params2[] = { { "foo", "one" }, { "foo", "two" } };
-static const struct qparamParseDataEntry const params3[] = { { "foo", "&one" }, { "bar", "&two" } };
-static const struct qparamParseDataEntry const params4[] = { { "foo", "" } };
-static const struct qparamParseDataEntry const params5[] = { { "foo", "one two" } };
-static const struct qparamParseDataEntry const params6[] = { { "foo", "one" } };
+static const struct qparamParseDataEntry params1[] = { { "foo", "one" }, { "bar", "two" } };
+static const struct qparamParseDataEntry params2[] = { { "foo", "one" }, { "foo", "two" } };
+static const struct qparamParseDataEntry params3[] = { { "foo", "&one" }, { "bar", "&two" } };
+static const struct qparamParseDataEntry params4[] = { { "foo", "" } };
+static const struct qparamParseDataEntry params5[] = { { "foo", "one two" } };
+static const struct qparamParseDataEntry params6[] = { { "foo", "one" } };
static int
mymain(int argc ATTRIBUTE_UNUSED,
--
1.6.1.2.443.g0d7c2
>From 75591fe809aaaf86d04c49cf314705bc4497a89f Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Sat, 31 Jan 2009 15:41:50 +0100
Subject: [PATCH 4/4] avoid a format-related warning
* src/qemu_driver.c (qemudStartVMDaemon): Use "%s".
---
src/qemu_driver.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 09f69bf..ebcdd88 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -1251,7 +1251,7 @@ static int qemudStartVMDaemon(virConnectPtr conn,
qemudLog(QEMUD_WARN, _("Domain %s didn't show up\n"), vm->def->name);
} else {
qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
- _("Unable to daemonize QEMU process"));
+ "%s", _("Unable to daemonize QEMU process"));
ret = -1;
}
}
--
1.6.1.2.443.g0d7c2
More information about the libvir-list
mailing list