httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeff Trawick <>
Subject Re: how to log apr_status_t diagnostics in mod_ssl...
Date Wed, 15 May 2002 12:11:07 GMT
"Ryan Bloom" <> writes:

> > From: [mailto:trawick@rdu88-251-
> >] 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

  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 |
Born in Roswell... married an alien...

View raw message