httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bh...@pobox.com
Subject cvs commit: apache-apr/pthreads/src/main http_protocol.c
Date Tue, 02 Mar 1999 21:39:01 GMT

This doesn't look right to me.

rbb@hyperreal.org writes:
> rbb         99/02/26 08:37:22
> 
>   Modified:    pthreads/src/main http_protocol.c
>   Log:
>   Inserted some error handling code, and got rid of a nasty infinite loop.  Stupid
>   errno not getting reset when I thought it was.  This should fix the timeout
>   code.

First it's almost never approprate [1] to set errno to zero.  It's not even in
the errno's value set.  

Second it's almost never approprate to try and get errno to hold a value for
more than a few instructions.  It's absolutely hopeless to call a function and
expect it to survive the call.  For example ap_bfileno might have dubug or
loging hooks in it and these might have errno setters in thier recursive decent.

Having had my interest drawn to this code...  It looks pretty racing assuming
that read_request_line left anything useful in errno when it returns zero.  Very
careful inspection of the code is probably necessary to be absolutely sure.  It
doesn't look very safe to me...

read_request_line might return the errno from getline.
getline might be returning the errno that from ap_bgets.

The behavior of ap_bgets is a mess in the -1 return case.  
It sets errno only on some of it's -1 return paths.

Bleck.

I think that the buff routines signal error by:
 1. Invoking the installed call back.
 2. Marking the buffer as having had an error.
 3. Returning an exceptional value, but _not_ setting errno.

Hope this is useful.

 - ben

[1] The only case I know of where it is approprate to set it to zero
is when you are debugging and your trying to get some hint if anything
"exceptional" happened recently.


Mime
View raw message