Guido Günther <agx(a)sigxcpu.org&gt; wrote: Good idea! Always welcome. Thanks! ACK, with one suggestion: ... 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: From: Jim Meyering <meyering(a)redhat.com&gt; Date: Sat, 31 Jan 2009 12:15:54 +0100 Subject: [PATCH] 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 f8740af..6eef328 100644 --- a/src/remote_internal.c +++ b/src/remote_internal.c @@ -5392,9 +5392,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.418.gd79e6

Jim Meyering <jim(a)meyering.net&gt; wrote: 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: Jim Meyering <meyering(a)redhat.com&gt; 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: Jim Meyering <meyering(a)redhat.com&gt; 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: Jim Meyering <meyering(a)redhat.com&gt; 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: Jim Meyering <meyering(a)redhat.com&gt; 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