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

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.