Re: [libvirt] [PATCH] usbFindBusByVendor: don't leak a DIR buffer and FD
On Tue, Jan 26, 2010 at 01:19:18PM -0500, Laine Stump wrote:
On 01/26/2010 10:04 AM, Daniel P. Berrange wrote:
>Yep, generally speaking if a caller needs to be given back the actual
>errno value, then the function should be returing '-errno' instead
>of the fixed -1.
>
-errno? Interesting. I recently fixed calls to virFileMakePath which
were looking for return < 0 to look for return != 0, because it returns
the value of errno (not -errno) in case of an error. Should I have
changed virFileMakePath to return -errno instead? All of the negations
through different paths in multiple layers might start to get confusing...
To be fair, it isn't actually clearcut one way or the other at this time.
There's a fairly even balance between functions returning 'errno' and
'-errno'. 95% of them just return '-1' though, and report errors directly
via one of virReportErrorXXXX functions. We should only return errno in
cases where the caller needs to be able to see and ignore the failure
scenarios.
Since 95% of cases use the simple 'return -1' for errors, my preference
is that we use 'return -errno' too, since that means we have a fairly
standard pattern of
if (foo() < 0) {
...blah...
}
or
if ((err = foo()) < 0) {
...blah...
}
for detecting errors when 'int' is the return type.
We should probably document this in the HACKING file and then change
an exceptions to match, but its not urgent.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|