httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From r..@covalent.net
Subject Re: Add hook to get server read.
Date Thu, 11 May 2000 19:34:21 GMT

> > Please, make comments quickly, because I am likely to commit quickly if
> > too many other changes start going into the tree.
> 
> Are people working in this area? (outside of CHANGES, that is)  I don't
> see a need to blindly rush...

The rush is that this is a large patch, and it touches http_core and
http_config, and touches them in a big way.  This is not a fun patch, so I
want it in before I have to re-write it.

Okay, I found a better way to do some stuff, so I'll be submitting a
different patch either later today or tomorrow.

BTW, more comments below.

Ryan

> >...
> > +static const char * ap_build_cont_config(ap_pool_t *p, ap_pool_t *temp_pool,
> 
> What does this function do? Can we get a one-line comment or something? It
> seems to reproduce a good amount of code from elsewhere. It looks like it
> sucks up lines from a config file, creating directives. But it doesn't
> process the EXEC_ON_READ directives like the "real" processing stuff.

It builds a sub tree for the directives within a container.

> > +    if (cmd = ap_find_command_in_modules(cmd_name, &mod)) {
> > +        if (cmd->req_override & EXEC_ON_READ) {
> > +            const char *retval;
> > +            ap_directive_t *sub_tree = NULL;
> > +
> > +            retval = execute_now(cmd_name, args, parms, p, temp_pool, 
> > +                                 &sub_tree, *curr_parent);
> > +            (*current)->next = sub_tree;
> > +            while ((*current)->next != NULL) {
> > +                (*current) = (*current)->next;
> > +            }
> 
> What's going on here? Looks like a subtree can be returned from the
> execution?

Yes, execute now returns a sub_tree which can be put back into the
tree.  This is how the code to get rid of the branches for ifmodule and
ifdefine works.

> > +static const char *execute_now(char *cmd_line, const char *args, cmd_parms *parms,

> > +                         ap_pool_t *p, ap_pool_t *ptemp, 
> > +                         ap_directive_t **sub_tree, ap_directive_t *parent)
> > +{
> > +    module *mod = top_module;
> > +    const command_rec *cmd;
> > +
> > +    if (!(cmd = ap_find_command_in_modules(cmd_line, &mod))) {
> > +        return ap_pstrcat(parms->pool, "Invalid command '", 
> > +                          cmd_line,
> > +                          "', perhaps mis-spelled or defined by a module "
> > +                          "not included in the server configuration",
> > +                          NULL);
> > +    }
> > +    else {
> > +        void *mconfig = ap_set_config_vectors(parms, 
> > +                                parms->server->lookup_defaults, mod);
> > +        const char *retval;
> > +
> > +        retval = invoke_cmd(cmd, parms, mconfig, args);
> > +        if (retval == NULL) {
> > +            int one = 1;
> 
> What is this "one" variable?

cruft from an earlier draft.

> 
> > +            if (*(int *)mconfig == 1) {
> 
> Woah... what is this magic here?

mconfig is a void *.  This casts it to a int * so that I can de-reference
it.  Take a look at the <FilesMatch directive which passes a (void*)1 to a
function.  This is how you would access that variable.

> Can there be a comment somwhere that simply states "an EXEC_ON_READ
> returns a boolean on whether the interior block should be processed?" or
> something like that. It seems like that is what is happening.

This is where things can and should be changed.  This will be obvious in a
while.  :-)

> >...
> > @@ -2245,10 +2258,10 @@
> >    RSRC_CONF|ACCESS_CONF, TAKE1,
> >    "How to work out the ServerName : Port when constructing URLs" },
> >  /* TODO: RlimitFoo should all be part of mod_cgi, not in the core */
> > -{ "AddModule", add_module_command, NULL, RSRC_CONF, ITERATE,
> > +{ "AddModule", add_module_command, NULL, RSRC_CONF | EXEC_ON_READ, ITERATE,
> >    "The name of a module" },
> > -{ "ClearModuleList", clear_module_list_command, NULL, RSRC_CONF, NO_ARGS, 
> > -  NULL },
> > +{ "!ClearModuleList", clear_module_list_command, NULL, RSRC_CONF | EXEC_ON_READ,
> 
> Is that exclamation mark intentional?

The first cut used !"directive" to signify that the directive should be
executed at read.  This was changed at the end to use EXEC_ON_READ.  I
thought I got them all, obviously I hadn't this is gone now.

> >  static const command_rec so_cmds[] = {
> > -    { "LoadModule", load_module, NULL, RSRC_CONF, TAKE2,
> > +    { "LoadModule", load_module, EXEC_ON_READ, RSRC_CONF, TAKE2,
> 
> The EXEC_ON_READ is in the wrong place. Should be or'd with RSRC_CONF.

Your right.  sorry.

> Could you repost the updated patch? I'm still not clear on how some of the
> pieces fit together and would like to understand a bit more.

As soon as I re-create some parts of it.

Ryan

_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------



Mime
View raw message