From: Laine Stump <laine@laine.org>
Date: Mon, 25 Jul 2011 18:11:38 +0000 (-0400)
Subject: util: change virFile*Pid functions to return < 0 on failure
X-Git-Tag: v0.9.4-rc1~8
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d6354c1696eae48afb2f5f46f12195f340108c72;p=thirdparty%2Flibvirt.git

util: change virFile*Pid functions to return < 0 on failure

Although most functions in libvirt return 0 on success and < 0 on
failure, there are a few functions lingering around that return errno
(a positive value) on failure, and sometimes code calling those
functions incorrectly assumes the <0 standard. I noticed one of these
the other day when auditing networkStartDhcpDaemon after Guido Gunther
found a place where success was improperly returned on failure (that
patch has been acked and is pending a push). The problem was that it
expected the return value from virFileReadPid to be < 0 on failure,
but it was actually positive (it was also neglected to set the return
code in this case, similar to the bug found by Guido).

This all led to the fact that *all* of the virFile*Pid functions in
util.c are returning errno on failure. This patch remedies that
problem by changing them all to return -errno on failure, and makes
any necessary changes to callers of the functions. (In the meantime, I
also properly set the return code on failure of virFileReadPid in
networkStartDhcpDaemon).
---

diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 7eda7ef276..ff42aa569a 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -947,8 +947,8 @@ int main(int argc, char *argv[])
             goto cleanup;
 
         if (pid > 0) {
-            if ((rc = virFileWritePid(LXC_STATE_DIR, name, pid)) != 0) {
-                virReportSystemError(rc,
+            if ((rc = virFileWritePid(LXC_STATE_DIR, name, pid)) < 0) {
+                virReportSystemError(-rc,
                                      _("Unable to write pid file '%s/%s.pid'"),
                                      LXC_STATE_DIR, name);
                 _exit(1);
@@ -996,5 +996,5 @@ cleanup:
         unlink(sockpath);
     VIR_FREE(sockpath);
 
-    return rc;
+    return rc ? EXIT_FAILURE : EXIT_SUCCESS;
 }
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 615d2c647b..7d99d27ee8 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1612,8 +1612,8 @@ static int lxcVmStart(virConnectPtr conn,
         goto cleanup;
 
     /* And get its pid */
-    if ((r = virFileReadPid(driver->stateDir, vm->def->name, &vm->pid)) != 0) {
-        virReportSystemError(r,
+    if ((r = virFileReadPid(driver->stateDir, vm->def->name, &vm->pid)) < 0) {
+        virReportSystemError(-r,
                              _("Failed to read pid file %s/%s.pid"),
                              driver->stateDir, vm->def->name);
         goto cleanup;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index a21b538a5d..b1c6b12bf3 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -758,8 +758,9 @@ networkStartDhcpDaemon(virNetworkObjPtr network)
      * pid
      */
 
-    if (virFileReadPid(NETWORK_PID_DIR, network->def->name,
-                       &network->dnsmasqPid) < 0)
+    ret = virFileReadPid(NETWORK_PID_DIR, network->def->name,
+                         &network->dnsmasqPid);
+    if (ret < 0)
         goto cleanup;
 
     ret = 0;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 646215e0ac..b87c32060f 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2754,7 +2754,7 @@ int qemuProcessStart(virConnectPtr conn,
 
     /* wait for qemu process to show up */
     if (ret == 0) {
-        if (virFileReadPidPath(priv->pidfile, &vm->pid)) {
+        if (virFileReadPidPath(priv->pidfile, &vm->pid) < 0) {
             qemuReportError(VIR_ERR_INTERNAL_ERROR,
                             _("Domain %s didn't show up"), vm->def->name);
             ret = -1;
diff --git a/src/util/command.c b/src/util/command.c
index 29ccaa4d7f..475eb62857 100644
--- a/src/util/command.c
+++ b/src/util/command.c
@@ -493,7 +493,7 @@ virExecWithHook(const char *const*argv,
         }
 
         if (pid > 0) {
-            if (pidfile && virFileWritePidPath(pidfile,pid)) {
+            if (pidfile && (virFileWritePidPath(pidfile,pid) < 0)) {
                 kill(pid, SIGTERM);
                 usleep(500*1000);
                 kill(pid, SIGTERM);
diff --git a/src/util/util.c b/src/util/util.c
index d83215ccc0..03a9e1adcd 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -1165,17 +1165,17 @@ int virFileWritePid(const char *dir,
     char *pidfile = NULL;
 
     if (name == NULL || dir == NULL) {
-        rc = EINVAL;
+        rc = -EINVAL;
         goto cleanup;
     }
 
     if (virFileMakePath(dir) < 0) {
-        rc = errno;
+        rc = -errno;
         goto cleanup;
     }
 
     if (!(pidfile = virFilePid(dir, name))) {
-        rc = ENOMEM;
+        rc = -ENOMEM;
         goto cleanup;
     }
 
@@ -1196,18 +1196,18 @@ int virFileWritePidPath(const char *pidfile,
     if ((fd = open(pidfile,
                    O_WRONLY | O_CREAT | O_TRUNC,
                    S_IRUSR | S_IWUSR)) < 0) {
-        rc = errno;
+        rc = -errno;
         goto cleanup;
     }
 
     if (!(file = VIR_FDOPEN(fd, "w"))) {
-        rc = errno;
+        rc = -errno;
         VIR_FORCE_CLOSE(fd);
         goto cleanup;
     }
 
     if (fprintf(file, "%d", pid) < 0) {
-        rc = errno;
+        rc = -errno;
         goto cleanup;
     }
 
@@ -1215,7 +1215,7 @@ int virFileWritePidPath(const char *pidfile,
 
 cleanup:
     if (VIR_FCLOSE(file) < 0)
-        rc = errno;
+        rc = -errno;
 
     return rc;
 }
@@ -1230,18 +1230,18 @@ int virFileReadPidPath(const char *path,
     *pid = 0;
 
     if (!(file = fopen(path, "r"))) {
-        rc = errno;
+        rc = -errno;
         goto cleanup;
     }
 
     if (fscanf(file, "%d", pid) != 1) {
-        rc = EINVAL;
+        rc = -EINVAL;
         VIR_FORCE_FCLOSE(file);
         goto cleanup;
     }
 
     if (VIR_FCLOSE(file) < 0) {
-        rc = errno;
+        rc = -errno;
         goto cleanup;
     }
 
@@ -1261,12 +1261,12 @@ int virFileReadPid(const char *dir,
     *pid = 0;
 
     if (name == NULL || dir == NULL) {
-        rc = EINVAL;
+        rc = -EINVAL;
         goto cleanup;
     }
 
     if (!(pidfile = virFilePid(dir, name))) {
-        rc = ENOMEM;
+        rc = -ENOMEM;
         goto cleanup;
     }
 
@@ -1284,17 +1284,17 @@ int virFileDeletePid(const char *dir,
     char *pidfile = NULL;
 
     if (name == NULL || dir == NULL) {
-        rc = EINVAL;
+        rc = -EINVAL;
         goto cleanup;
     }
 
     if (!(pidfile = virFilePid(dir, name))) {
-        rc = ENOMEM;
+        rc = -ENOMEM;
         goto cleanup;
     }
 
     if (unlink(pidfile) < 0 && errno != ENOENT)
-        rc = errno;
+        rc = -errno;
 
 cleanup:
     VIR_FREE(pidfile);
diff --git a/tests/commandtest.c b/tests/commandtest.c
index 9ab446c363..ef2850d257 100644
--- a/tests/commandtest.c
+++ b/tests/commandtest.c
@@ -230,7 +230,7 @@ static int test4(const void *unused ATTRIBUTE_UNUSED)
         goto cleanup;
     }
 
-    if (virFileReadPid(abs_builddir, "commandhelper", &pid) != 0) {
+    if (virFileReadPid(abs_builddir, "commandhelper", &pid) < 0) {
         printf("cannot read pidfile\n");
         goto cleanup;
     }
@@ -686,7 +686,7 @@ static int test18(const void *unused ATTRIBUTE_UNUSED)
     }
     alarm(0);
 
-    if (virFileReadPid(abs_builddir, "commandhelper", &pid) != 0) {
+    if (virFileReadPid(abs_builddir, "commandhelper", &pid) < 0) {
         printf("cannot read pidfile\n");
         goto cleanup;
     }