You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2010/07/20 08:57:37 UTC
Re: svn commit: r965408 - in /httpd/httpd/trunk: include/ap_mmn.h
include/http_config.h modules/core/mod_so.c server/config.c
On 07/19/2010 12:06 PM, sf@apache.org wrote:
> Author: sf
> Date: Mon Jul 19 10:06:15 2010
> New Revision: 965408
>
> URL: http://svn.apache.org/viewvc?rev=965408&view=rev
> Log:
> Add ap_find_module_short_name() to quickly get the module short name
> (i.e. symbol name with trailing "_module" removed) from the module_index.
> To be used for logging.
>
> Modified:
> httpd/httpd/trunk/include/ap_mmn.h
> httpd/httpd/trunk/include/http_config.h
> httpd/httpd/trunk/modules/core/mod_so.c
> httpd/httpd/trunk/server/config.c
>
> Modified: httpd/httpd/trunk/server/config.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/config.c?rev=965408&r1=965407&r2=965408&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/config.c (original)
> +++ httpd/httpd/trunk/server/config.c Mon Jul 19 10:06:15 2010
> @@ -549,8 +555,30 @@ AP_DECLARE(const char *) ap_add_module(m
> "reached. Please increase "
> "DYNAMIC_MODULE_LIMIT and recompile.", m->name);
> }
> +
> + }
> + else if (!sym_name) {
> + while (sym->modp != NULL) {
> + if (sym->modp == m) {
> + sym_name = sym->name;
> + break;
> + }
> + sym++;
> + }
> }
>
> + if (sym_name) {
> + int len = strlen(sym_name);
> + int slen = strlen("_module");
> + if (len > slen && !strcmp(sym_name + len - slen, "_module")) {
> + len -= slen;
> + }
> +
> + ap_module_short_names[m->module_index] = strdup(sym_name);
Why not using pools here instead of malloc / free?
> + ap_module_short_names[m->module_index][len] = '\0';
> + }
> +
> +
> /* Some C compilers put a complete path into __FILE__, but we want
> * only the filename (e.g. mod_includes.c). So check for path
> * components (Unix and DOS), and remove them.
> @@ -623,13 +651,17 @@ AP_DECLARE(void) ap_remove_module(module
> modp->next = modp->next->next;
> }
>
> + free(ap_module_short_names[m->module_index]);
Why not using pools here instead of malloc / free?
> + ap_module_short_names[m->module_index] = NULL;
> +
> m->module_index = -1; /* simulate being unloaded, should
> * be unnecessary */
> dynamic_modules--;
> total_modules--;
> }
>
> -AP_DECLARE(const char *) ap_add_loaded_module(module *mod, apr_pool_t *p)
> +AP_DECLARE(const char *) ap_add_loaded_module(module *mod, apr_pool_t *p,
> + const char *short_name)
> {
> module **m;
> const char *error;
Regards
Rüdiger
Re: svn commit: r965408 - in /httpd/httpd/trunk: include/ap_mmn.h
include/http_config.h modules/core/mod_so.c server/config.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 07/20/2010 08:43 PM, Stefan Fritsch wrote:
> On Tuesday 20 July 2010, Ruediger Pluem wrote:
>> On 07/19/2010 12:06 PM, sf@apache.org wrote:
>>> +
>>> + ap_module_short_names[m->module_index] =
>>> strdup(sym_name);
>> Why not using pools here instead of malloc / free?
>
> Because it didn't work. But looking at the code again, I believe this
> is due to ap_setup_prelinked_modules calling ap_add_module with the
> wrong pool. ap_setup_prelinked_modules is called only once and should
> therefore use process->pool instead of process->pconf. Do you agree?
>
>
Not sure, because ap_add_module calls
ap_add_module_commands(m, p);
/* FIXME: is this the right place to call this?
* It doesn't appear to be
*/
ap_register_hooks(m, p);
with this pool. So this would need closer investigation if
process->pool is correct for them as well in this case.
But yes, if this fits process->pool is the correct choice IMHO.
Regards
Rüdiger
Re: svn commit: r965408 - in /httpd/httpd/trunk: include/ap_mmn.h include/http_config.h modules/core/mod_so.c server/config.c
Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Tuesday 20 July 2010, Ruediger Pluem wrote:
> On 07/19/2010 12:06 PM, sf@apache.org wrote:
> > Author: sf
> > Date: Mon Jul 19 10:06:15 2010
> > New Revision: 965408
> >
> > URL: http://svn.apache.org/viewvc?rev=965408&view=rev
> > Log:
> > Add ap_find_module_short_name() to quickly get the module short
> > name (i.e. symbol name with trailing "_module" removed) from the
> > module_index. To be used for logging.
> >
> > Modified:
> > httpd/httpd/trunk/include/ap_mmn.h
> > httpd/httpd/trunk/include/http_config.h
> > httpd/httpd/trunk/modules/core/mod_so.c
> > httpd/httpd/trunk/server/config.c
> >
> > Modified: httpd/httpd/trunk/server/config.c
> > URL:
> > http://svn.apache.org/viewvc/httpd/httpd/trunk/server/config.c?r
> > ev=965408&r1=965407&r2=965408&view=diff
> > ================================================================
> > ============== --- httpd/httpd/trunk/server/config.c (original)
> > +++ httpd/httpd/trunk/server/config.c Mon Jul 19 10:06:15 2010
> >
> > @@ -549,8 +555,30 @@ AP_DECLARE(const char *) ap_add_module(m
> >
> > "reached. Please increase "
> > "DYNAMIC_MODULE_LIMIT and
> > recompile.", m->name);
> >
> > }
> >
> > +
> > + }
> > + else if (!sym_name) {
> > + while (sym->modp != NULL) {
> > + if (sym->modp == m) {
> > + sym_name = sym->name;
> > + break;
> > + }
> > + sym++;
> > + }
> >
> > }
> >
> > + if (sym_name) {
> > + int len = strlen(sym_name);
> > + int slen = strlen("_module");
> > + if (len > slen && !strcmp(sym_name + len - slen,
> > "_module")) { + len -= slen;
> > + }
> > +
> > + ap_module_short_names[m->module_index] =
> > strdup(sym_name);
>
> Why not using pools here instead of malloc / free?
Because it didn't work. But looking at the code again, I believe this
is due to ap_setup_prelinked_modules calling ap_add_module with the
wrong pool. ap_setup_prelinked_modules is called only once and should
therefore use process->pool instead of process->pconf. Do you agree?