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

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 :|


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