httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jtra...@covalent.net
Subject Re: concerns about the recent error checking added to APR
Date Thu, 06 Apr 2000 03:04:13 GMT
Jeff,

I hope this mail goes out correctly, I'm away from my work computer.
I agree with you on many of your points, namely that if we check
for the NULL input, and return BADARG, etc. that we are:

	1 - Slowing the program down
	2 - Denying the programmer a nice SEGV

First, wrt #2, I'd say that this is a really nice thing for us as
developers to have.  The SEGV and associated feedback about program
state are infinitely more useful than a 'BADARG', etc.  My response
to this would be to perhaps use debug and production assertions for
checking of these values.  This would give end users a meaningful
error message if the case arises, instead of SEGV, etc., in addition
to giving developers the chance to pull it into their debugger.  

wrt #1, this would indeed slow the program down slightly, but what is the
cost of a check on a pointer compared to system calls for opening files,
etc.?  I'd perhaps like to see numbers before throwing out error
checking in a production library.  Not everyone running apache will
be a programmer, and not everyone will have source code lying around.

I had tossed out ideas for checking validity of input types before (
other than NUL), but I think anyone I proposed the idea to seemed to 
think the idea was not worth the effort for the gain.

I would be very happy with production level code with with checks at
the interface layers.  Obviously, once things get internal it is
a bit redundant.

Finally, the patch I submitted with all these changes in it had some
other bugfixes in there.  I'd maybe like to see those get in before
we worry much more about error checking. ;-)

-- Jon


>Jeff writes:
>
>I just backed out (perhaps temporarily :) ) a change to
>ap_open_stderr() which caused it to fail if it was passed NULL for
>cont.
>
>The immediate reason is that apache can't log to stderr at startup
>with that code.  Not even "httpd -h" will work.
>There is storage allocated and it will be malloc()-ed if no context is
>passed.  Do you really want to prevent the app from being able to do
>that?  If so, then make sure apache can log to stderr at startup when
>this is re-implemented.
>
>Some of the other error checking (e.g., return APR_EBADARG if some
>parameter is NULL) looks nice on the surface, but it is truly bad.
>An app developer should hit the places where bad parms were passed to
>APR in the course of their normal testing.
>If you return a bad return code, it can be hidden because not all APR
>calls are checked for errors by all apps.  The app may very well
>continue with no apparent side-effects.
>
>If you blow up by dereferencing a NULL pointer, the developer
>definitely finds out about it and they have the means to debug the
>problem, even if they don't check return codes and/or don't have a way
>to trace failures.
>
>Even after the developer is done with the code, if they leave some
>untested paths which pass bad parms to APR, it is still hard to
>justify APR returning a bad return code instead of letting the
>segfaults fall where they may.  If the developer didn't test that path
>in the first place, there is no reason to think that there is an error
>path to handle APR_EBADARG in an appropriate manner.  It is better to
>go out with a boom than to silently fail.
>
>Note that a check for NULL is a check for only one of quite a few
>different invalid pointers :) , so its usefulness is quite limited to
>start with.
>
>Meanwhile all the code is slightly slower.
>
>On the other hand...
>. I'm not so sure that this is an appropriate strategy for APIs for
>  which sources aren't available to help the debugger diagnose a core
>  dump.  The supplier of object code only APIs can't spin its wheels
>  looking at core dumps for app developers who pass bad parms to the
>  API.
>. I'm not so sure this is an appropriate strategy for APIs which gain
>  authority on entry to do their business.  (But these will have
>  better mechanisms for accessing user storage carefully.)
>. I remain quite interested in the handling of error codes that we can
>  actually expect at run-time due to the environment or due to user
>  actions.
>
>

Mime
View raw message