Re: errno handling

[Carles Pina i Estany]

Hi,

On Nov/22/2009, Carles Pina i Estany wrote:
> 
> Hello,
> 
> On Nov/22/2009, Robert Millan wrote:
> > On Sun, Nov 22, 2009 at 05:19:19PM +0000, Carles Pina i Estany wrote:
> > > 
> > > Hello,
> > > 
> > > Last night I spent some time with a confussion that maybe it can be
> > > avoided (or maybe not).
> > > 
> > > Let's say that a module calls grub_file_open and it fails (file doesn't
> > > exist or whatever). grub_errno is setted up.
> > > 
> > > This module is not resetting grub_errno and makes another call to
> > > grub_file_open with a valid file. But grub_file_open does:
> > 
> > In theory, each layer should check errno after a call that may be
> > setting it.  It's up to the layer whether errno has to be ignored
> > (and reset), handled or passed to the upper layer.
> 
> In my opinion grub_file_open should reset grub_errno because
> grub_file_open behaviour is different up to the grub_errno when it
> enters in the function (if grub_errno != GRUB_ERRNO then grub_file_open
> will not open any file). And this for more functions.

discussed in irc with phcoder: no changes here because would be a caller
bug. Some day I will take a look on a possible callers that are not
resetting grub_errno after functions that are setting up (so can lead to
problems later)

The discussion:

 <phcoder> cpina: there is nothing wrong with red herring on bugs for
 the sake of kernel size

 <cpina> phcoder: so you would not touch grub_file_open to keep the size
 as small as possible in the kernel and move this code to the users of
 grub_file_open?

 <cpina> to keep the kernel size as you can understand :-)

 <phcoder> cpina: it's not what I said. Caller has to handle error
 return. If it doesn't it's a bug. If caller bugs core is allowed to do
 anything it wants

 <cpina> phcoder: i understand, but when i design api's i try to make it
 hard for the users to miss-use the api. And in this case it's easy to
 miss-use the api.

 <phcoder> cpina: with grub we have other requirements. If it really
 bothers you so much you can add assertions which would be enabled with
 ./configure --debug-mode

...

Cheers,

-- 
Carles Pina i Estany
        http://pinux.info