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

Re: [libvirt] [PATCHv2 4/4] Remove erroneous setting of return value to errno.



 On 07/21/2010 11:37 AM, Eric Blake wrote:
On 07/21/2010 09:21 AM, Daniel Veillard wrote:
   ACK, but it seems to me ret being initialized to -1, maybe it's better
to be consistent and on all error do a ret = -errno; before
virReportSystemError and the goto cleanup.
If you have the convention of returning -errno, then you must not return
-1 unless you meant to report EPERM.  I agree with Laine's approach of
initializing to 0 in this case, since you don't want to leak an
unintended -1 if that was not the real error.


In the case of this function, the patch isn't intended to make the function return -errno (like the others in this series), it was just to make it consistent; this was a bug that I coincidentally noticed while changing the other functions from "return positive errno on failure" to "return -errno on failure". I think DV was wondering out loud if we should change the convention of this function (from "return -1 on error") as well. In order to do that, I would have to 1) change all other functions that are called interchangeably with this function (ie, anything assigned to create_func in storage_backend_fs.c, assigned to .buildVolFrom, or a couple other places), and 2) assure that all places any of these are called are okay with a return that is merely < 0, rather than explicitly -1. (and we would also need to make sure that there was a reasonable errno to return in every error case; for example in some functions the error is caused by a configuration problem, not by some system failure).

Instead of pushing this change right away, I'll go through all those functions to see if such a change is possible.


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