You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Joe Orton <jo...@redhat.com> on 2004/08/11 17:24:28 UTC

[PATCH] add test_config hook

Any interest in this for HEAD?  Patch below adds a test_config hook
which allows modules to do extra checking when httpd is invoked with -t.
The new "-t -D DUMP_MODULES" feature can be implemented this way rather
than hooking into mod_so from server/main.c, which I think is slightly
cleaner; it's useful for other modules too.

Index: server/config.c
===================================================================
RCS file: /home/cvs/httpd-2.0/server/config.c,v
retrieving revision 1.180
diff -u -r1.180 config.c
--- server/config.c	16 Jul 2004 20:30:43 -0000	1.180
+++ server/config.c	11 Aug 2004 15:22:03 -0000
@@ -72,6 +72,7 @@
            APR_HOOK_LINK(handler)
            APR_HOOK_LINK(quick_handler)
            APR_HOOK_LINK(optional_fn_retrieve)
+           APR_HOOK_LINK(test_config)
 )
 
 AP_IMPLEMENT_HOOK_RUN_ALL(int, header_parser,
@@ -81,6 +82,10 @@
                           (apr_pool_t *pconf, apr_pool_t *plog,
                            apr_pool_t *ptemp),
                           (pconf, plog, ptemp), OK, DECLINED)
+
+AP_IMPLEMENT_HOOK_VOID(test_config,
+                       (apr_pool_t *pconf, server_rec *s),
+                       (pconf, s))
 
 AP_IMPLEMENT_HOOK_RUN_ALL(int, post_config,
                           (apr_pool_t *pconf, apr_pool_t *plog,
Index: server/main.c
===================================================================
RCS file: /home/cvs/httpd-2.0/server/main.c,v
retrieving revision 1.161
diff -u -r1.161 main.c
--- server/main.c	12 Jul 2004 16:36:42 -0000	1.161
+++ server/main.c	11 Aug 2004 15:22:03 -0000
@@ -37,8 +37,6 @@
 #include "ap_mpm.h"
 #include "mpm_common.h"
 
-#include "mod_so.h" /* for dumping loaded modules */
-
 /* WARNING: Win32 binds http_main.c dynamically to the server. Please place
  *          extern functions and global data in another appropriate module.
  *
@@ -609,18 +607,12 @@
     rv = ap_process_config_tree(server_conf, ap_conftree,
                                 process->pconf, ptemp);
     if (rv == OK) {
-        APR_OPTIONAL_FN_TYPE(ap_dump_loaded_modules) *dump_modules =
-            APR_RETRIEVE_OPTIONAL_FN(ap_dump_loaded_modules);
-        
         ap_fixup_virtual_hosts(pconf, server_conf);
         ap_fini_vhost_config(pconf, server_conf);
         apr_hook_sort_all();
 
-        if (dump_modules && ap_exists_config_define("DUMP_MODULES")) {
-            dump_modules(pconf, server_conf);
-        }
-
         if (configtestonly) {
+            ap_run_test_config(pconf, server_conf);
             ap_log_error(APLOG_MARK, APLOG_STARTUP, 0, NULL, "Syntax OK");
             destroy_and_exit_process(process, 0);
         }
Index: include/http_config.h
===================================================================
RCS file: /home/cvs/httpd-2.0/include/http_config.h,v
retrieving revision 1.111
diff -u -r1.111 http_config.h
--- include/http_config.h	14 Jul 2004 06:36:41 -0000	1.111
+++ include/http_config.h	11 Aug 2004 15:22:04 -0000
@@ -967,6 +967,13 @@
 AP_DECLARE_HOOK(int,pre_config,(apr_pool_t *pconf,apr_pool_t *plog,
                                 apr_pool_t *ptemp))
 
+/**
+ * Run the test_config function for each module; this hook is run
+ * only if the server was invoked to test the configuration syntax.
+ * @param pconf The config pool
+ * @param s The list of server_recs
+ */
+AP_DECLARE_HOOK(void,test_config,(apr_pool_t *pconf, server_rec *s))
 
 /**
  * Run the post_config function for each module
Index: modules/mappers/mod_so.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/mappers/mod_so.c,v
retrieving revision 1.59
diff -u -r1.59 mod_so.c
--- modules/mappers/mod_so.c	12 Jul 2004 16:36:42 -0000	1.59
+++ modules/mappers/mod_so.c	11 Aug 2004 15:22:04 -0000
@@ -347,13 +347,18 @@
     return NULL;
 }
 
-static void ap_dump_loaded_modules(apr_pool_t* p, server_rec* s)
+static void dump_loaded_modules(apr_pool_t* p, server_rec* s)
 {
     ap_module_symbol_t *modie;
     ap_module_symbol_t *modi;
     so_server_conf *sconf;
     int i;
     apr_file_t *out = NULL;
+
+    if (!ap_exists_config_define("DUMP_MODULES")) {
+        return;
+    }
+
     apr_file_open_stderr(&out, p);
 
     apr_file_printf(out, "Loaded Modules:\n");
@@ -402,7 +407,7 @@
 {
 #ifndef NO_DLOPEN
     APR_REGISTER_OPTIONAL_FN(ap_find_loaded_module_symbol);
-    APR_REGISTER_OPTIONAL_FN(ap_dump_loaded_modules);
+    ap_hook_test_config(dump_loaded_modules, NULL, NULL, APR_HOOK_MIDDLE);
 #endif
 }
 

Re: [PATCH] add test_config hook

Posted by Paul Querna <ch...@force-elite.com>.
On Wed, 2004-08-11 at 16:24 +0100, Joe Orton wrote:
> Any interest in this for HEAD?  Patch below adds a test_config hook
> which allows modules to do extra checking when httpd is invoked with -t.
> The new "-t -D DUMP_MODULES" feature can be implemented this way rather
> than hooking into mod_so from server/main.c, which I think is slightly
> cleaner; it's useful for other modules too.

Yes, I wrote the Dump Modules stuff, and I like your method better. It
is *much* cleaner.

If there aren't any objections, I will commit this in a couple days.

-Paul Querna


> Index: server/config.c
> ===================================================================
> RCS file: /home/cvs/httpd-2.0/server/config.c,v
> retrieving revision 1.180
> diff -u -r1.180 config.c
> --- server/config.c	16 Jul 2004 20:30:43 -0000	1.180
> +++ server/config.c	11 Aug 2004 15:22:03 -0000
> @@ -72,6 +72,7 @@
>             APR_HOOK_LINK(handler)
>             APR_HOOK_LINK(quick_handler)
>             APR_HOOK_LINK(optional_fn_retrieve)
> +           APR_HOOK_LINK(test_config)
>  )
>  
>  AP_IMPLEMENT_HOOK_RUN_ALL(int, header_parser,
> @@ -81,6 +82,10 @@
>                            (apr_pool_t *pconf, apr_pool_t *plog,
>                             apr_pool_t *ptemp),
>                            (pconf, plog, ptemp), OK, DECLINED)
> +
> +AP_IMPLEMENT_HOOK_VOID(test_config,
> +                       (apr_pool_t *pconf, server_rec *s),
> +                       (pconf, s))
>  
>  AP_IMPLEMENT_HOOK_RUN_ALL(int, post_config,
>                            (apr_pool_t *pconf, apr_pool_t *plog,
> Index: server/main.c
> ===================================================================
> RCS file: /home/cvs/httpd-2.0/server/main.c,v
> retrieving revision 1.161
> diff -u -r1.161 main.c
> --- server/main.c	12 Jul 2004 16:36:42 -0000	1.161
> +++ server/main.c	11 Aug 2004 15:22:03 -0000
> @@ -37,8 +37,6 @@
>  #include "ap_mpm.h"
>  #include "mpm_common.h"
>  
> -#include "mod_so.h" /* for dumping loaded modules */
> -
>  /* WARNING: Win32 binds http_main.c dynamically to the server. Please place
>   *          extern functions and global data in another appropriate module.
>   *
> @@ -609,18 +607,12 @@
>      rv = ap_process_config_tree(server_conf, ap_conftree,
>                                  process->pconf, ptemp);
>      if (rv == OK) {
> -        APR_OPTIONAL_FN_TYPE(ap_dump_loaded_modules) *dump_modules =
> -            APR_RETRIEVE_OPTIONAL_FN(ap_dump_loaded_modules);
> -        
>          ap_fixup_virtual_hosts(pconf, server_conf);
>          ap_fini_vhost_config(pconf, server_conf);
>          apr_hook_sort_all();
>  
> -        if (dump_modules && ap_exists_config_define("DUMP_MODULES")) {
> -            dump_modules(pconf, server_conf);
> -        }
> -
>          if (configtestonly) {
> +            ap_run_test_config(pconf, server_conf);
>              ap_log_error(APLOG_MARK, APLOG_STARTUP, 0, NULL, "Syntax OK");
>              destroy_and_exit_process(process, 0);
>          }
> Index: include/http_config.h
> ===================================================================
> RCS file: /home/cvs/httpd-2.0/include/http_config.h,v
> retrieving revision 1.111
> diff -u -r1.111 http_config.h
> --- include/http_config.h	14 Jul 2004 06:36:41 -0000	1.111
> +++ include/http_config.h	11 Aug 2004 15:22:04 -0000
> @@ -967,6 +967,13 @@
>  AP_DECLARE_HOOK(int,pre_config,(apr_pool_t *pconf,apr_pool_t *plog,
>                                  apr_pool_t *ptemp))
>  
> +/**
> + * Run the test_config function for each module; this hook is run
> + * only if the server was invoked to test the configuration syntax.
> + * @param pconf The config pool
> + * @param s The list of server_recs
> + */
> +AP_DECLARE_HOOK(void,test_config,(apr_pool_t *pconf, server_rec *s))
>  
>  /**
>   * Run the post_config function for each module
> Index: modules/mappers/mod_so.c
> ===================================================================
> RCS file: /home/cvs/httpd-2.0/modules/mappers/mod_so.c,v
> retrieving revision 1.59
> diff -u -r1.59 mod_so.c
> --- modules/mappers/mod_so.c	12 Jul 2004 16:36:42 -0000	1.59
> +++ modules/mappers/mod_so.c	11 Aug 2004 15:22:04 -0000
> @@ -347,13 +347,18 @@
>      return NULL;
>  }
>  
> -static void ap_dump_loaded_modules(apr_pool_t* p, server_rec* s)
> +static void dump_loaded_modules(apr_pool_t* p, server_rec* s)
>  {
>      ap_module_symbol_t *modie;
>      ap_module_symbol_t *modi;
>      so_server_conf *sconf;
>      int i;
>      apr_file_t *out = NULL;
> +
> +    if (!ap_exists_config_define("DUMP_MODULES")) {
> +        return;
> +    }
> +
>      apr_file_open_stderr(&out, p);
>  
>      apr_file_printf(out, "Loaded Modules:\n");
> @@ -402,7 +407,7 @@
>  {
>  #ifndef NO_DLOPEN
>      APR_REGISTER_OPTIONAL_FN(ap_find_loaded_module_symbol);
> -    APR_REGISTER_OPTIONAL_FN(ap_dump_loaded_modules);
> +    ap_hook_test_config(dump_loaded_modules, NULL, NULL, APR_HOOK_MIDDLE);
>  #endif
>  }
>  


Re: [PATCH] add test_config hook

Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
--On Wednesday, August 11, 2004 4:24 PM +0100 Joe Orton <jo...@redhat.com> 
wrote:

> Any interest in this for HEAD?  Patch below adds a test_config hook
> which allows modules to do extra checking when httpd is invoked with -t.
> The new "-t -D DUMP_MODULES" feature can be implemented this way rather
> than hooking into mod_so from server/main.c, which I think is slightly
> cleaner; it's useful for other modules too.

Looks fine.  +1.  -- justin