Various fixes following a code review

* src/libvirt.c src/lxc/lxc_conf.c src/lxc/lxc_container.c
  src/lxc/lxc_controller.c src/node_device/node_device_hal.c
  src/openvz/openvz_conf.c src/qemu/qemu_driver.c
  src/qemu/qemu_monitor_text.c src/remote/remote_driver.c
  src/storage/storage_backend_disk.c src/storage/storage_driver.c
  src/util/logging.c src/xen/sexpr.c src/xen/xend_internal.c
  src/xen/xm_internal.c: Steve Grubb <sgrubb@redhat.com> sent a code
  review and those are the fixes correcting the problems

14 files changed, 46 insertions, 45 deletions

diff --git a/src/libvirt.c b/src/libvirt.c
index 2c5079088..9e80e29ef 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -1122,7 +1122,7 @@ do_open (const char *name,
               (res == VIR_DRV_OPEN_DECLINED ? "DECLINED" :
                (res == VIR_DRV_OPEN_ERROR ? "ERROR" : "unknown status")));
         if (res == VIR_DRV_OPEN_ERROR) {
-            if (0 && STREQ(virStorageDriverTab[i]->name, "remote")) {
+            if (STREQ(virStorageDriverTab[i]->name, "remote")) {
                 virLibConnWarning (NULL, VIR_WAR_NO_STORAGE,
                                    "Is the daemon running ?");
             }
diff --git a/src/lxc/lxc_conf.c b/src/lxc/lxc_conf.c
index 74dc36747..bc815432e 100644
--- a/src/lxc/lxc_conf.c
+++ b/src/lxc/lxc_conf.c
@@ -31,6 +31,7 @@
 #include "nodeinfo.h"
 #include "virterror_internal.h"
 #include "conf.h"
+#include "memory.h"
 #include "logging.h"
 
 
@@ -111,10 +112,10 @@ int lxcLoadDriverConfig(lxc_driver_t *driver)
 
     /* Avoid error from non-existant or unreadable file. */
     if (access (filename, R_OK) == -1)
-        return 0;
+        goto done;
     conf = virConfReadFile(filename, 0);
     if (!conf)
-        return 0;
+        goto done;
 
     p = virConfGetValue(conf, "log_with_libvirtd");
     if (p) {
@@ -125,6 +126,9 @@ int lxcLoadDriverConfig(lxc_driver_t *driver)
     }
 
     virConfFree(conf);
+
+done:
+    VIR_FREE(filename);
     return 0;
 
 no_memory:
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 97b7903a5..c77d0990e 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -753,6 +753,7 @@ static int lxcContainerChild( void *data )
         virReportSystemError(NULL, errno,
                              _("Failed to open tty %s"),
                              ttyPath);
+        VIR_FREE(ttyPath);
         return -1;
     }
     VIR_FREE(ttyPath);
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 0b104a173..3cecdceda 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -501,7 +501,7 @@ lxcControllerRun(virDomainDefPtr def,
 {
     int rc = -1;
     int control[2] = { -1, -1};
-    int containerPty;
+    int containerPty = -1;
     char *containerPtyPath;
     pid_t container = -1;
     virDomainFSDefPtr root;
@@ -734,7 +734,7 @@ int main(int argc, char *argv[])
         goto cleanup;
     }
 
-    if (getuid() && 0) {
+    if (getuid() != 0) {
         fprintf(stderr, "%s: must be run as the 'root' user\n", argv[0]);
         goto cleanup;
     }
diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c
index fe8d11669..818c7d69d 100644
--- a/src/node_device/node_device_hal.c
+++ b/src/node_device/node_device_hal.c
@@ -364,7 +364,7 @@ static int gather_capabilities(LibHalContext *ctx, const char *udi,
 {
     char *bus_name = NULL;
     virNodeDevCapsDefPtr caps = NULL;
-    char **hal_cap_names;
+    char **hal_cap_names = NULL;
     int rv, i;
 
     if (STREQ(udi, "/org/freedesktop/Hal/devices/computer")) {
@@ -778,11 +778,6 @@ static int halDeviceMonitorStartup(int privileged ATTRIBUTE_UNUSED)
     virNodeDeviceObjListFree(&driverState->devs);
     if (hal_ctx)
         (void)libhal_ctx_free(hal_ctx);
-    if (udi) {
-        for (i = 0; i < num_devs; i++)
-            VIR_FREE(udi[i]);
-        VIR_FREE(udi);
-    }
     nodeDeviceUnlock(driverState);
     VIR_FREE(driverState);
 
diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c
index f4c62f33f..a7ccfdb84 100644
--- a/src/openvz/openvz_conf.c
+++ b/src/openvz/openvz_conf.c
@@ -329,12 +329,14 @@ openvz_replace(const char* str,
                const char* to) {
     const char* offset = NULL;
     const char* str_start = str;
-    int to_len = strlen(to);
-    int from_len = strlen(from);
+    int to_len;
+    int from_len;
     virBuffer buf = VIR_BUFFER_INITIALIZER;
 
-    if(!from)
+    if ((!from) || (!to))
         return NULL;
+    from_len = strlen(from);
+    to_len = strlen(to);
 
     while((offset = strstr(str_start, from)))
     {
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 245abb731..7d20a545d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1180,9 +1180,9 @@ qemudReadLogOutput(virConnectPtr conn,
         usleep(100*1000);
         retries--;
     }
-    if (retries == 0)
-        qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
-                         _("Timed out while reading %s log output"), what);
+
+    qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+                     _("Timed out while reading %s log output"), what);
     return -1;
 }
 
@@ -4635,6 +4635,7 @@ static int qemudDomainChangeEjectableMedia(virConnectPtr conn,
         newdisk->src = NULL;
         origdisk->type = newdisk->type;
     }
+    VIR_FREE(devname);
 
     return ret;
 }
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index c866111e9..ba111d872 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -937,9 +937,10 @@ doRemoteOpen (virConnectPtr conn,
         if (priv->pid > 0) {
             pid_t reap;
             do {
+retry:
                 reap = waitpid(priv->pid, NULL, 0);
                 if (reap == -1 && errno == EINTR)
-                    continue;
+                    goto retry;
             } while (reap != -1 && reap != priv->pid);
         }
 #endif
@@ -1400,9 +1401,10 @@ doRemoteClose (virConnectPtr conn, struct private_data *priv)
     if (priv->pid > 0) {
         pid_t reap;
         do {
+retry:
             reap = waitpid(priv->pid, NULL, 0);
             if (reap == -1 && errno == EINTR)
-                continue;
+                goto retry;
         } while (reap != -1 && reap != priv->pid);
     }
 #endif
diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c
index e82959cca..72ccd8167 100644
--- a/src/storage/storage_backend_disk.c
+++ b/src/storage/storage_backend_disk.c
@@ -82,12 +82,10 @@ virStorageBackendDiskMakeDataVol(virConnectPtr conn,
          * dir every time its run. Should figure out a more efficient
          * way of doing this...
          */
-        if ((vol->target.path = virStorageBackendStablePath(conn,
-                                                            pool,
-                                                            devpath)) == NULL)
-            return -1;
-
+        vol->target.path = virStorageBackendStablePath(conn, pool, devpath);
         VIR_FREE(devpath);
+        if (vol->target.path == NULL)
+            return -1;
     }
 
     if (vol->key == NULL) {
@@ -646,7 +644,7 @@ virStorageBackendDiskDeleteVol(virConnectPtr conn,
 
     part_num = devname + strlen(srcname);
 
-    if (!part_num) {
+    if (*part_num == 0) {
         virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
                               _("cannot parse partition number from target "
                                 "'%s'"), devname);
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 80e4543f9..e15de28c2 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -80,7 +80,8 @@ storageDriverAutostart(virStorageDriverStatePtr driver) {
                 backend->startPool(NULL, pool) < 0) {
                 virErrorPtr err = virGetLastError();
                 storageLog("Failed to autostart storage pool '%s': %s",
-                           pool->def->name, err ? err->message : NULL);
+                           pool->def->name, err ? err->message :
+                           "no error message found");
                 virStoragePoolObjUnlock(pool);
                 continue;
             }
@@ -90,7 +91,8 @@ storageDriverAutostart(virStorageDriverStatePtr driver) {
                 if (backend->stopPool)
                     backend->stopPool(NULL, pool);
                 storageLog("Failed to autostart storage pool '%s': %s",
-                           pool->def->name, err ? err->message : NULL);
+                           pool->def->name, err ? err->message :
+                           "no error message found");
                 virStoragePoolObjUnlock(pool);
                 continue;
             }
diff --git a/src/util/logging.c b/src/util/logging.c
index 4899de6f5..757f78cce 100644
--- a/src/util/logging.c
+++ b/src/util/logging.c
@@ -386,7 +386,7 @@ int virLogDefineFilter(const char *match, int priority,
     }
 
     mdup = strdup(match);
-    if (dup == NULL) {
+    if (mdup == NULL) {
         i = -1;
         goto cleanup;
     }
@@ -484,6 +484,7 @@ int virLogDefineOutput(virLogOutputFunc f, virLogCloseFunc c, void *data,
 
     virLogLock();
     if (VIR_REALLOC_N(virLogOutputs, virLogNbOutputs + 1)) {
+        VIR_FREE(ndup);
         goto cleanup;
     }
     ret = virLogNbOutputs++;
diff --git a/src/xen/sexpr.c b/src/xen/sexpr.c
index 81cb49f26..085500dc5 100644
--- a/src/xen/sexpr.c
+++ b/src/xen/sexpr.c
@@ -440,9 +440,6 @@ sexpr_lookup_key(const struct sexpr *sexpr, const char *node)
     for (token = strsep(&ptr, "/"); token; token = strsep(&ptr, "/")) {
         const struct sexpr *i;
 
-        if (token == NULL)
-            continue;
-
         sexpr = sexpr->u.s.cdr;
         for (i = sexpr; i->kind != SEXPR_NIL; i = i->u.s.cdr) {
             if (i->kind != SEXPR_CONS ||
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index 3c660befb..dad07841a 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -1369,16 +1369,14 @@ xend_parse_sexp_desc_char(virConnectPtr conn,
                 goto no_memory;
         }
 
-        if (connectPort) {
-            if (connectHost) {
-                virBufferVSprintf(buf,
-                                  "      <source mode='connect' host='%s' service='%s'/>\n",
-                                  connectHost, connectPort);
-            } else {
-                virBufferVSprintf(buf,
-                                  "      <source mode='connect' service='%s'/>\n",
-                                  connectPort);
-            }
+        if (connectHost) {
+            virBufferVSprintf(buf,
+                              "      <source mode='connect' host='%s' service='%s'/>\n",
+                              connectHost, connectPort);
+        } else {
+            virBufferVSprintf(buf,
+                              "      <source mode='connect' service='%s'/>\n",
+                              connectPort);
         }
         if (bindPort) {
             if (bindHost) {
diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c
index 5878ba155..f833ce7ef 100644
--- a/src/xen/xm_internal.c
+++ b/src/xen/xm_internal.c
@@ -576,8 +576,7 @@ int xenXMConfigCacheRefresh (virConnectPtr conn) {
     virHashRemoveSet(priv->configCache, xenXMConfigReaper, xenXMConfigFree, &args);
     ret = 0;
 
-    if (dh)
-        closedir(dh);
+    closedir(dh);
 
     return (ret);
 }
@@ -2229,8 +2228,9 @@ virConfPtr xenXMDomainConfigFormat(virConnectPtr conn,
     if (xenXMConfigSetInt(conf, "vcpus", def->vcpus) < 0)
         goto no_memory;
 
-    if (def->cpumask &&
-        !(cpus = virDomainCpuSetFormat(conn, def->cpumask, def->cpumasklen)) < 0)
+    if ((def->cpumask != NULL) &&
+        ((cpus = virDomainCpuSetFormat(conn, def->cpumask,
+                                       def->cpumasklen)) == NULL))
         goto cleanup;
 
     if (cpus &&