"Ryan Bloom" <[EMAIL PROTECTED]> writes:
> > From: [EMAIL PROTECTED] [mailto:trawick@rdu88-251-
> > 253.nc.rr.com] On Behalf Of Jeff Trawick
> >
> > mod_ssl uses ssl_log(), which doesn't allow the caller to pass in
> > apr_status_t. Should we add a new parameter for that, or should we
> > add a separate function like ssl_log_error() with such a parameter?
> >
> > I'd vote for adding a new parm to ssl_log() right after the 2nd
> > parameter (and yes, I'll make the changes to the many callers :) ).
> >
> > Also, the existing accesses to errno in ssl_log() should be yanked, or
> > perhaps replaced with appropriate logic to handle whether or not an
> > apr_status_t was passed in.
> >
> > Comments?
>
> If you wanted to change all ssl_log
> calls to ap_log_[r]error, I would be +1 for that too.
If you look at ssl_log(), you'll find some pretty extensive support
for special features that ap_log_[r]error() and apr_strerror()
probably shouldn't ever have to know about.
Consider the SSL_ADD_SSLERR flag... It leads to a lookup and log
of one or more SSL errors. This definitely belongs in a utility
routine like ssl_log(). SSL_LOG_INIT is a less interesting feature
which saves some callers a bit of work (but not a critical feature).
This isn't a piece of code I'm very familiar with, and I would not be
at all shocked if there is complexity there that is not necessary in
real life, but for now it looks like the following is appropriate:
if special features are used like SSL_ADD_SSLERR:
keep calling ssl_log()
(it remains to be seen whether or not ssl_log() needs an
apr_status_t parameter; I doubt it since the SSL libraries don't
use APR and APR doesn't make room for an SSL library error code in
apr_status_t)
if ssl_log(...SSL_LOG_TRACE...)
-> ap_log_error(...APLOG_DEBUG...)
if ssl_log(...SSL_LOG_ERR...)
-> ap_log_error(...APLOG_ERR...)
Meanwhile rip out the support for tracing to a separate file since
that is effectively broken with the translation above.
--
Jeff Trawick | [EMAIL PROTECTED]
Born in Roswell... married an alien...
