httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jon Travis <jtra...@covalent.net>
Subject Re: DSO Patch 1 of 3 (+ commentary!)
Date Mon, 27 Mar 2000 19:04:10 GMT
rbb@apache.org wrote:

> nn> This suprises me a little, since APR likes to use opaque data types.  Obviously
> > assertions can be used for much more than simple opaque type-checking, but that
> > was my main concern.  The apathy towards zealous error-checking concerns me a
> > little bit, especially with very new development code && opaque data.
>
> The asserts are extraneous, and don't belong there.  If you look at ALL of
> the APR code, we check for valid data before trying to execute the
> function.  The asserts are only useful while debugging the code.  We want
> error messages if bad data is passed.

The assertions definitely DO belong there.  By examination of many other APR calls, we
do no examination of the data at an entry point level (interface layer), and since
many of these API calls are at the interface level with modules, etc. we must do our
own error checking internally.   Since you point out that 'ALL' the APR code checks
for valid data before trying to execute subsequent functions, let me give you an
example or 2 where we do not:

In apr_pools.c, in ap_alloc, we simply de-reference the context, and pass in a value
to ap_pool_alloc, which for all we know, may be a server config structure for some
arbitrary module.  What kind of error checking or printing of error message is that?
Most likely you will get some kind of segmentation violation shortly thereafter.  Or
an even more basic check in ap_vformatter, where we do not even do a simple check that
vbuff is not NULL.

What I am proposing is a solution which will enable us to do run-time checking of
stupidity errors (i.e. passing in of the wrong opaque type, etc.)  I do not write
bug-free code, and I think there may be just 1 or 2 other people in the world who
don't write bug-free code either, so I think it may be nice for the 3 of us to get
some real error messages about run-time issues.  This isn't anything new or
revolutionary.  Writing defensive code cuts down on the debug time drastically.  And
you are correct, the assertions are useful for debugging the code -- and since we are
in an 'alpha', I think this is something we should be doing a lot of.



> > > Error strings do not belong in APR functions.  There are multiple ways to
> > > get error strings out of APR (none of them are written yet).  ap_strerror,
> >
> > > which uses strerror when it is a system error and generates it's own error
> >
> > > string when it is an APR error or status value, is the preferred method.
> > > For some APR types, an ap_foo_error function which provides an error
> > > string could be written.  This function should be added in extremely rare
> > > cases, and great care needs to be taken when adding it.
> > >
> > > Ryan
> >
> > Well, I think dlerror() falls under neither of the above catagories (errno system
> > error, or an internal APR type error), so perhaps the v-- below comment may be
> > appropriate.
> >
> > Perhaps some generic facilities could be put into place that would be more
> > thread-safe, etc.  I.e. ap_set_lasterror() and ap_get_lasterror(), which would
> > return that information in a per-thread basis.  I'd be perfectly content with
> > something like this.
>
> Again, this isn't necessary.  I looked at the original code, and all of
> the error strings were "UNKNOWN ERROR", expect for one "Failed to get
> image symbol".  If we add an APR error code for this case, this can easily
> be handled by ap_strerror.
>
> Ryan

Look again -- you are sorely mistaken.  In the BeOS code that I 'wrote', there are
those conditions -- solely because I have no BeOS development API from which to get
information about error results.  However, take a quick glance at the unix and os/2
files that I wrote, and you can see that there are error strings which your
ap_strerror() are simply not going to account for.  You do not necessarily know all
the internals of these low-level operations (i.e. exactly all the strings they will
return, etc), so you still need some way of giving out error information.

-- Jon


Mime
View raw message