Jim Meyering <jim@meyering.net> wrote:
> Guido Günther <agx@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@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@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@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@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@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