httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Stein <gst...@lyra.org>
Subject Re: Add hook to get server read.
Date Thu, 11 May 2000 22:46:48 GMT
On Thu, 11 May 2000 rbb@covalent.net wrote:
>...
> Okay, I found a better way to do some stuff, so I'll be submitting a
> different patch either later today or tomorrow.

Cool. I'm out of town from Friday late morning until Sunday afternoon. If
you get it done before I leave: sweet; if not, then I'll review when I get
back (whether you've held it or checked it in... I can find it :-)  I'd
prefer if you held it, but it's your call, of course.

>...
> > > +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.

Understood, but as I mentioned... it seems like we already have code for
that purpose. And there is the lack of EXEC_ON_READ; if that is
intentional, then maybe we can refactor the "build a container" code to
take a flag on whether to obey EXEC_ON_READ.

I can see at least two types of execution:

1) build tree and call EXEC_ON_READ
2) throw out a container (due to a filed conditional like IfModule)

I'm confused about the build with no EXEC_ON_READ.

> > > +    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.

I think you mean "when ifmodule/ifdefine want to *include* the branch,
then they return the sub_tree." Correct?

>...
> > 
> > > +            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.

I understand that, but I don't see the other side. The part where we set
up the integer and then examine it.

> > 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.  :-)

Okay :-)

>...
> > > -{ "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.

No problem! That's what review is for :-)

>...
> > 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.

Coolness. I look forward to the new set, to see your "new idea". I presume
it cleans/clarifies :-)

But... no rush! I'm just Mr Peanut Gallery right now :-)

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/



Mime
View raw message