Re: [PATCH] util: change virFile*Pid functions to return < 0 on failure

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07/25/2011 03:17 PM, Eric Blake wrote:
On 07/25/2011 12:19 PM, Laine Stump wrote:
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).

+++ b/src/lxc/lxc_controller.c

@@ -996,5 +996,5 @@ cleanup:
          unlink(sockpath);
      VIR_FREE(sockpath);

-    return rc;
+ return -rc; /* rc is 0 or negative, but returns from main should be>=0 */

This hunk is wrong. errno can validly be a multiple of 256, in which case this results in the program exiting with status 0 instead of an error. There's no portable way to use errno as a program's exit status. Rather, this should be:

return rc ? EXIT_FAILURE : EXIT_SUCCESS;

so that we are guaranteed that main() exits with a non-zero positive value on failure that cannot wrap around.

ACK to the patch with that problem fixed.



Thanks! I made the change and pushed.

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]