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.
>...
> --- mod_log_config.c 2001/04/18 03:53:33 1.53
> +++ mod_log_config.c 2001/04/18 21:06:07 1.54
>...
> +static void ap_register_log_handler(apr_pool_t *p, char *tag,
> + ap_log_handler_fn_t *handler, int def)
> +{
> + ap_log_handler *log_struct = apr_palloc(p, sizeof(*log_struct));
> + log_struct->func = handler;
> + log_struct->want_orig_default = def;
> +
> + apr_hash_set(log_hash, tag, 1, (const void *)log_struct);
> +}
> +
> +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?
> 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).
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)
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
