On Wed, 18 Apr 2001, Greg Stein wrote:
> On Wed, Apr 18, 2001 at 09:06:07PM -0000, [EMAIL PROTECTED] wrote:
> >...
> > --- http_core.c 2001/04/18 03:53:30 1.271
> > +++ http_core.c 2001/04/18 21:06:06 1.272
> > @@ -59,6 +59,7 @@
> > #include "apr_strings.h"
> > #include "apr_thread_proc.h" /* for RLIMIT stuff */
> > #include "apr_lib.h"
> > +#include "apr_optional.h"
> >
> > #define APR_WANT_STRFUNC
> > #include "apr_want.h"
> > @@ -77,6 +78,7 @@
> > #include "scoreboard.h"
> >
> > #include "mod_core.h"
> > +#include "../loggers/mod_log_config.h"
>
> I'd say that modules/loggers/ should be added to the INCLUDES path so we
> don't need this relative path. mod_dav and mod_http (core) does this. Maybe
> for mod_include, too?
>
> Remember to install mod_log_config.h, too.
Yeah, I just followed what was there already. We need to figure out how
we are going to do this stuff.
> > +static void log_pre_config(apr_pool_t *p, apr_pool_t *plog, apr_pool_t *ptemp)
> > +{
> > + static APR_OPTIONAL_FN_TYPE(ap_register_log_handler) *log_pfn_register;
> > +
> > + log_hash = apr_hash_make(p);
> > + log_pfn_register = APR_RETRIEVE_OPTIONAL_FN(ap_register_log_handler);
> > +
> > + if (log_pfn_register) {
>
> Why this monkey work? Why aren't you just calling ap_register_log_handler
> directly? There isn't any reason to do it via the OPTIONAL_FN stuff since
> the function is *in* this file.
>
> For clarity/maintainability, can't we just call directly?
Same reason we do it this way in mod_include. This shows people how to do
it themselves, and it is a REALLY fast way to see when things break. We
should always be eating our own dog food, otherwise we are asking to miss
problems.
> > static void register_hooks(apr_pool_t *p)
> > {
> > + ap_hook_pre_config(log_pre_config,NULL,NULL,APR_HOOK_REALLY_FIRST);
>
> I'd suggest just using APR_HOOK_MIDDLE. People that are using this module
> should be using the predecessor stuff, as you did in http_core.
>
> Any time we see "REALLY_FIRST", we should raise a red flag. That generally
> means something is going wrong (that we aren't using the hooks facilities as
> they were originally intended).
We can do that. This was a part of debugging the problem with the hook
ordering, and I forgot about it.
> Anyways... great patch. It hides that ap_http_conn_rec properly. Now... can
> we make that structure private? :-)
>
> (personally, I'd hope to see mod_core.h (mod_http) totally private)
I agree, it should be totally private. Feel free to make ap_http_conn_rec
private, or just wait until we can make the whole file private.
Ryan
_______________________________________________________________________________
Ryan Bloom [EMAIL PROTECTED]
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
