httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
Subject Re: concerns about the recent error checking added to APR
Date Thu, 06 Apr 2000 14:37:04 GMT

> I just backed out (perhaps temporarily :) ) a change to
> ap_open_stderr() which caused it to fail if it was passed NULL for
> cont. 

I'll take full responsability for those changes.  When I reviewed the
patch, I forgot we allocated memory using malloc if the context is NULL.
The part of this patch which checks a context for NULL needs to be backed
out, and I'll take care of that today.

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

I'm not sure I agree with this.  I'm not sure I disagree either.  I am
torn on this issue.

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

This is just a bad idea, and any programmer who gets burned because they
didn't check an error code deserves what they get.  :-)

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

I think some error checking is necessary here, but I would like to keep it
at a minimum.

Expect the if (cont == NULL) checks to be removed VERY soon.


View raw message