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 21:29:47 GMT
On Thu, 11 May 2000 rbb@covalent.net wrote:
>...
> 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...

>...
> diff -u -d -b -w -u -r1.109 CHANGES
> --- src/CHANGES	2000/05/11 20:25:38	1.109
> +++ src/CHANGES	2000/05/11 20:46:36
> @@ -1,8 +1,21 @@
> +<<<<<<< CHANGES
> +Changes with Apache 2.0a4-dev
> +  *) Add the ability to hook into the config file reading phase.  Basically
> +     if a directive is specified EXEC_ON_READ, then when that directive is
> +     read from the config file, the assocaited function is executed.  This
> +     should only be used for those directives that must muck with HOW the
> +     server INTERPRETS the config.  This should not be used for directives
> +     that re-order or replace items in the config tree.  Those changes should
> +     be made in the pre-config step.
> +     [Ryan Bloom]
> +
> +=======
>  Changes with Apache 2.0a4
>  
>    *) Add mod_example to the build system.
>       [Tony Finch]
>  
> +>>>>>>> 1.109

Merge conflict!

>...
> @@ -155,6 +166,7 @@
>      int override;		/* Which allow-override bits are set */
>      int limited;		/* Which methods are <Limit>ed */
>  
> +    configfile_t *config_file;  /* Config file structure. */

I don't like seeing this back in there. It simply continues to propagate
the notion of "config comes from a file". But, I don't have a better
solution at this point. If I come up with something, then I'll deal with
that later.

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

>... ap_build_cont_config ...
> +        }
> +    }
> +}

Missing a return value.


>  static const char * ap_build_config_sub(ap_pool_t *p, ap_pool_t *temp_pool,
> -					const configfile_t *cfp,
> -					const char *l,
> +					const char *l, cmd_parms *parms,
>  					ap_directive_t **current,
>  					ap_directive_t **curr_parent)
>  {
>      const char *args;
>      char *cmd_name;
>      ap_directive_t *newdir;
> +    module *mod = top_module;
> +    const command_rec *cmd;
>  
>      if (*l == '#' || *l == '\0')
>  	return NULL;
> @@ -857,9 +922,24 @@
>  	return NULL;
>      }
>  
> +    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?

>...
> @@ -1064,6 +1144,71 @@
>      return ap_make_full_path(p, ap_server_root, file);
>  }
>  
> +API_EXPORT(const char *) ap_soak_end_container(cmd_parms *cmd, char *directive)
> +{
> +    char l[MAX_STRING_LEN];
> +    const char *args;
> +    char *cmd_name;
> +
> +    while(!(ap_cfg_getline(l, MAX_STRING_LEN, cmd->config_file))) {
> +#if RESOLVE_ENV_PER_TOKEN
> +        args = l;
> +#else
> +        args = ap_resolve_env(cmd->temp_pool, l);
> +#endif
> +        cmd_name = ap_getword_conf(cmd->pool, &args);
> +        if (cmd_name[0] == '<') {
> +            if (cmd_name[1] == '/') {
> +                char *bracket = '\0';
> +                if (strcasecmp(cmd_name + 2,
> +                                directive + 1) != 0) {
> +                    return ap_pstrcat(cmd->pool, "Expected </",
> +                                      directive + 1, "> but saw ",
> +                                      cmd_name, ">", NULL);
> +                }
> +                break;
> +            }
> +            else {
> +                ap_soak_end_container(cmd, cmd_name);

Check the return value!

> +            }
> +        }
> +    }

Missing a return value again.

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

> +            if (*(int *)mconfig == 1) {

Woah... what is this magic here?

> +                return ap_build_cont_config(p, ptemp, parms, sub_tree, &parent,
cmd_line);
> +            }
> +            else {
> +                ap_soak_end_container(parms, cmd_line);

Check the return value.

> +                return retval;
> +            }
> +        }
> +        if (strcmp(retval, DECLINE_CMD) != 0)
> +            return retval;
> +        }
> +}

Missing a return value again.

>...
> Index: src/main/http_core.c
> ===================================================================
> RCS file: /home/cvs/apache-2.0/src/main/http_core.c,v
> retrieving revision 1.56
> diff -u -d -b -w -u -r1.56 http_core.c
> --- src/main/http_core.c	2000/05/11 02:32:05	1.56
> +++ src/main/http_core.c	2000/05/11 20:46:41
> @@ -1539,6 +1539,7 @@
>  {
>      char *endp = strrchr(arg, '>');
>      int not = (arg[0] == '!');
> +    int one = 1;

Why the 'one' variable? Just assign constants to 'dummy'.

>      module *found;
>  
>      if (endp == NULL) {
> @@ -1554,8 +1555,14 @@
>      found = ap_find_linked_module(arg);
>  
>      if ((!not && found) || (not && !found)) {
> -        return ap_walk_config(cmd->directive->first_child, cmd, cmd->context);
> +        *(int *)dummy = one;
> +        return NULL;
>      }
> +    else { 
> +        one = 0;
> +        *(int *)dummy = one;
> +        return NULL;
> +    }

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.

>...
> @@ -1578,6 +1585,7 @@
>  {
>      char *endp;
>      int defined;
> +    int one = 1;

Again with this 'one' thing.

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

>...
> --- src/modules/standard/mod_so.c	2000/04/27 04:15:48	1.14
> +++ src/modules/standard/mod_so.c	2000/05/11 20:46:42
> @@ -349,7 +349,7 @@
>  #endif /* NO_DLOPEN */
>  
>  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.


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.

thx!
-g

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


Mime
View raw message