httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From David Robinson <d...@esi.co.uk>
Subject Re: errnos and buff.c
Date Wed, 16 Dec 1998 09:07:41 GMT
The author of buff.c writes...

On Tue, 15 Dec 1998, Marc Slemko wrote:

> Right now, in numerous places, we assume that buff.c functions will have a
> valid errno if they return -1.  In some cases, they do set an errno.  
> 
> But in others they don't.  They just leave any old errno there.  
> ...
> 
> in any case case, ap_bwrite checks if there was an error:
> 
> 1207        if (fb->flags & (B_WRERR | B_EOUT))
> 
> and, if so, just returns -1 without touching errno.  errno happened to be
> EAGAIN from some previous IO, so we get screwed.
> 
> There are other problems with ap_send_fb_length WRT not checking return
> values, but those are other problems.
> 
> So does anyone know what the goal was?  First, I think the useless EAGAIN
> checks (ie. checks in places where we don't deal with nonblocking
> descriptors anyway) should be removed.  

I don't agree. The code was written to make as few assumptions as
possible; in particular, it will function if it is given non-blocking file
descriptors. It is defensive programming; the code should consider an
EAGAIN error even is that is not expected.

> Second, either functions have to always set errno when they return an
> error or never set it when they return an error.  Which one is it to be?
> And what errno can we set for a generic error when we don't have the real
> errno?

I think (but can recall for certain) that the doerror() function and the
(fb->flags & (B_WRERR | B_EOUT)) construct was used to ensure that
the routines will 'work' even in the application does not check the error
return for every call. It was also to provide an error hook which is
only to be invoked once for an error.

I'm not au fait with how Apache uses these routines now, but I would
suggest one of two solutions:

1. Remove doerror() and the test at the start of each routine.

As long as you don't use the error hook, and do check every return code

2. Change the code to do

doerror(BUFF *fb)
{
   fb->berrno = errno
   ...
}

and in each routine, for example:
    if (fb->flags & B_RDERR)
    {
        errno = fb->berrno;
        return -1;
    }


David Robinson





Mime
View raw message