You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by pq...@apache.org on 2005/05/29 23:39:40 UTC
svn commit: r178990 - in /httpd/httpd/trunk: CHANGES server/config.c
Author: pquerna
Date: Sun May 29 14:39:39 2005
New Revision: 178990
URL: http://svn.apache.org/viewcvs?rev=178990&view=rev
Log:
- Use a hash for the configuration command lookup implementation. Now modules can properly directives without dependence on the LoadModule order.
Modified:
httpd/httpd/trunk/CHANGES
httpd/httpd/trunk/server/config.c
Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/CHANGES?rev=178990&r1=178989&r2=178990&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES (original)
+++ httpd/httpd/trunk/CHANGES Sun May 29 14:39:39 2005
@@ -2,6 +2,9 @@
[Remove entries to the current 2.0 section below, when backported]
+ *) core: Allow multiple modules to register interest in a single
+ configuration command. [Paul Querna]
+
*) EBCDIC: Handle chunked input from client or, with proxy, origin
server. [Jeff Trawick]
Modified: httpd/httpd/trunk/server/config.c
URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/server/config.c?rev=178990&r1=178989&r2=178990&view=diff
==============================================================================
--- httpd/httpd/trunk/server/config.c (original)
+++ httpd/httpd/trunk/server/config.c Sun May 29 14:39:39 2005
@@ -182,6 +182,8 @@
AP_DECLARE_DATA module *ap_top_module = NULL;
AP_DECLARE_DATA module **ap_loaded_modules=NULL;
+static apr_hash_t *ap_config_hash = NULL;
+
typedef int (*handler_func)(request_rec *);
typedef void *(*dir_maker_func)(apr_pool_t *, char *);
typedef void *(*merger_func)(apr_pool_t *, void *, void *);
@@ -408,6 +410,40 @@
}
}
+
+typedef struct ap_mod_list_struct ap_mod_list;
+struct ap_mod_list_struct {
+ struct ap_mod_list_struct *next;
+ module *m;
+ const command_rec *cmd;
+};
+
+static void ap_add_module_commands(module *m)
+{
+ apr_pool_t* tpool;
+ ap_mod_list* mln;
+ const command_rec *cmd;
+ char* dir;
+
+ cmd = m->cmds;
+
+ tpool = apr_hash_pool_get(ap_config_hash);
+
+ while (cmd && cmd->name) {
+ mln = apr_palloc(tpool, sizeof(ap_mod_list));
+ mln->cmd = cmd;
+ mln->m = m;
+ dir = apr_pstrdup(tpool, cmd->name);
+
+ ap_str_tolower(dir);
+
+ mln->next = apr_hash_get(ap_config_hash, dir, APR_HASH_KEY_STRING);
+ apr_hash_set(ap_config_hash, dir, APR_HASH_KEY_STRING, mln);
+ ++cmd;
+ }
+}
+
+
/* One-time setup for precompiled modules --- NOT to be done on restart */
AP_DECLARE(const char *) ap_add_module(module *m, apr_pool_t *p)
@@ -465,6 +501,7 @@
}
#endif /*_OSD_POSIX*/
+ ap_add_module_commands(m);
/* FIXME: is this the right place to call this?
* It doesn't appear to be
*/
@@ -585,6 +622,8 @@
apr_hook_global_pool=process->pconf;
+ ap_config_hash = apr_hash_make(process->pool);
+
/*
* Initialise total_modules variable and module indices
*/
@@ -1042,54 +1081,53 @@
cmd_parms *parms,
ap_conf_vector_t *section_vector)
{
- module *mod = ap_top_module;
+ const command_rec *cmd;
+ ap_mod_list* ml;
+ char* dir = apr_pstrdup(parms->pool, current->directive);
- while (1) {
- const command_rec *cmd;
+ ap_str_tolower(dir);
- if (!(cmd = ap_find_command_in_modules(current->directive, &mod))) {
- parms->err_directive = current;
- return apr_pstrcat(parms->pool, "Invalid command '",
- current->directive,
- "', perhaps misspelled or defined by a module "
- "not included in the server configuration",
- NULL);
- }
- else {
- void *dir_config = ap_set_config_vectors(parms->server,
- section_vector,
- parms->path,
- mod,
- parms->pool);
- const char *retval;
-
- /* Once was enough? */
- if (cmd->req_override & EXEC_ON_READ) {
- return NULL;
- }
+ ml = apr_hash_get(ap_config_hash, dir, APR_HASH_KEY_STRING);
- retval = invoke_cmd(cmd, parms, dir_config, current->args);
- if (retval == NULL) {
- return NULL;
- }
+ if (ml == NULL) {
+ parms->err_directive = current;
+ return apr_pstrcat(parms->pool, "Invalid command '",
+ current->directive,
+ "', perhaps misspelled or defined by a module "
+ "not included in the server configuration",
+ NULL);
+ }
- if (strcmp(retval, DECLINE_CMD) != 0) {
- /* If the directive in error has already been set, don't
- * replace it. Otherwise, an error inside a container
- * will be reported as occuring on the first line of the
- * container.
- */
- if (!parms->err_directive) {
- parms->err_directive = current;
- }
+ for ( ; ml != NULL; ml = ml->next) {
+ void *dir_config = ap_set_config_vectors(parms->server,
+ section_vector,
+ parms->path,
+ ml->m,
+ parms->pool);
+ const char *retval;
+ cmd = ml->cmd;
- return retval;
- }
+ /* Once was enough? */
+ if (cmd->req_override & EXEC_ON_READ) {
+ continue;
+ }
- mod = mod->next; /* Next time around, skip this one */
+ retval = invoke_cmd(cmd, parms, dir_config, current->args);
+
+ if (retval != NULL && strcmp(retval, DECLINE_CMD) != 0) {
+ /* If the directive in error has already been set, don't
+ * replace it. Otherwise, an error inside a container
+ * will be reported as occuring on the first line of the
+ * container.
+ */
+ if (!parms->err_directive) {
+ parms->err_directive = current;
+ }
+ return retval;
}
}
- /* NOTREACHED */
+
+ return NULL;
}
AP_DECLARE(const char *) ap_walk_config(ap_directive_t *current,
@@ -1332,19 +1370,34 @@
ap_directive_t **sub_tree,
ap_directive_t *parent)
{
- module *mod = ap_top_module;
const command_rec *cmd;
+ ap_mod_list* ml;
+ char* dir = apr_pstrdup(parms->pool, cmd_line);
+
+ ap_str_tolower(dir);
- if (!(cmd = ap_find_command_in_modules(cmd_line, &mod))) {
+ ml = apr_hash_get(ap_config_hash, dir, APR_HASH_KEY_STRING);
+
+ if (ml == NULL) {
return apr_pstrcat(parms->pool, "Invalid command '",
cmd_line,
"', perhaps misspelled or defined by a module "
"not included in the server configuration",
NULL);
}
- else {
- return invoke_cmd(cmd, parms, sub_tree, args);
+
+ for ( ; ml != NULL; ml = ml->next) {
+ const char *retval;
+ cmd = ml->cmd;
+
+ retval = invoke_cmd(cmd, parms, sub_tree, args);
+
+ if (retval != NULL) {
+ return retval;
+ }
}
+
+ return NULL;
}
/* This structure and the following functions are needed for the
Re: svn commit: r178990 - in /httpd/httpd/trunk: CHANGES server/config.c
Posted by Paul Querna <ch...@force-elite.com>.
Joe Orton wrote:
> On Thu, Jun 02, 2005 at 02:05:05PM +0100, Joe Orton wrote:
>
>>On Sun, May 29, 2005 at 09:39:40PM -0000, Paul Querna wrote:
>>
>>>Author: pquerna
>>>Date: Sun May 29 14:39:39 2005
>>>New Revision: 178990
>>>
>>>URL: http://svn.apache.org/viewcvs?rev=178990&view=rev
>>>Log:
>>>- Use a hash for the configuration command lookup implementation. Now
>>>modules can properly directives without dependence on the LoadModule
>>>order.
>>
>>It seems this has broken the test suite somehow, it's failing to start
>>up with mod_ssl issuing the error:
>>
>>[Thu Jun 02 14:01:14 2005] [error] Init: Multiple RSA server certificates not allowed
>>
>>if I revert this revision it works again. Any ideas?
>
>
> The problem is that the directive->module mapping is never getting
> removed when the DSO is unloaded, so during normal startup each
> directive will appear in the hash table twice for DSOs; then once more
> for each restart. Hence mod_ssl is confused.
>
> This is pretty dangerous since the module might *not* be reloaded again
> during a restart but the reference remains. And since everything is
> allocated out of the global pool it's leaking memory across restarts
> too.
Opps. Fixed in r179617. It now does everything out of the pconf pool,
which leads to some hackish code to re-add the prelinked modules, but it
all seems correct now on graceful restarts.
<dream>I really don't like how graceful works. Maybe after rewriting
the entire configuration system it will be better.</dream>
> plus much style badness for a third criticism, while I'm here ;)
>
> apr_pool_t* tpool;
> ap_mod_list* mln;
Fixed in r179618. I should almost write a pre-commit hook to catch
these for me, the * next to the type just seems more natural to me....
Thanks,
Paul
Re: svn commit: r178990 - in /httpd/httpd/trunk: CHANGES server/config.c
Posted by Joe Orton <jo...@redhat.com>.
On Thu, Jun 02, 2005 at 02:05:05PM +0100, Joe Orton wrote:
> On Sun, May 29, 2005 at 09:39:40PM -0000, Paul Querna wrote:
> > Author: pquerna
> > Date: Sun May 29 14:39:39 2005
> > New Revision: 178990
> >
> > URL: http://svn.apache.org/viewcvs?rev=178990&view=rev
> > Log:
> > - Use a hash for the configuration command lookup implementation. Now
> > modules can properly directives without dependence on the LoadModule
> > order.
>
> It seems this has broken the test suite somehow, it's failing to start
> up with mod_ssl issuing the error:
>
> [Thu Jun 02 14:01:14 2005] [error] Init: Multiple RSA server certificates not allowed
>
> if I revert this revision it works again. Any ideas?
The problem is that the directive->module mapping is never getting
removed when the DSO is unloaded, so during normal startup each
directive will appear in the hash table twice for DSOs; then once more
for each restart. Hence mod_ssl is confused.
This is pretty dangerous since the module might *not* be reloaded again
during a restart but the reference remains. And since everything is
allocated out of the global pool it's leaking memory across restarts
too.
plus much style badness for a third criticism, while I'm here ;)
apr_pool_t* tpool;
ap_mod_list* mln;
joe
Re: svn commit: r178990 - in /httpd/httpd/trunk: CHANGES server/config.c
Posted by Joe Orton <jo...@redhat.com>.
On Sun, May 29, 2005 at 09:39:40PM -0000, Paul Querna wrote:
> Author: pquerna
> Date: Sun May 29 14:39:39 2005
> New Revision: 178990
>
> URL: http://svn.apache.org/viewcvs?rev=178990&view=rev
> Log:
> - Use a hash for the configuration command lookup implementation. Now
> modules can properly directives without dependence on the LoadModule
> order.
It seems this has broken the test suite somehow, it's failing to start
up with mod_ssl issuing the error:
[Thu Jun 02 14:01:14 2005] [error] Init: Multiple RSA server certificates not allowed
if I revert this revision it works again. Any ideas?
joe