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.