httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From r..@covalent.net
Subject Re: New config work...
Date Sat, 03 Jun 2000 03:56:59 GMT

> If I understand this correctly, the overall strategy that you're going
> for here isn't going to quite work. When you enter ap_build_config(), the
> "current" must be initialized properly.
> 
> Hmm. Actually, it shouldn't be too much code:
> 
>   current = conftree;
>   if (current != NULL)
>     while (current->next != NULL)
>       current = current->next;

Actually, I did consider this, and I don't believe it is necessary.  Just
realized that it is important.  I'll add that code to the right spots
before I commit.

> Oh: Include directives' resulting trees are not hooked into the config
> tree for later execution. Before this patch, an Include directive would
> parse/exec the include file before returning. With this patch, it only
> parses the file :-)

The include file implementation is a bug then.  This should have been
fixed before.  I guess I forgot to make Include an exec_on_read.  I'll add
that too.

> >  CORE_EXPORT(const char *) ap_init_virtual_host(ap_pool_t *p, const char *hostname,
> >  				server_rec *main_server, server_rec **);
> > -void ap_process_resource_config(server_rec *s, const char *fname, ap_pool_t *p,
ap_pool_t *ptemp);
> > +ap_directive_t *ap_process_resource_config(server_rec *s, const char *fname, 
> > +                     ap_directive_t *conftree, ap_pool_t *p, ap_pool_t *ptemp);
> > +void ap_process_config_tree(server_rec *s, ap_directive_t *conftree,
> > +                            ap_pool_t *p, ap_pool_t *ptemp);
> 
> I think it would be better to have these use "ap_directive_t **conftree",
> and continue to return void to the caller.
> 
> I'd like to update them in the future to return the errmsg (rather than
> doing an exit()).
> 
> The **conftree pattern is also pretty common.

Fine.  I didn't really care.  I'll make the change.

> > +    errmsg = ap_walk_config(conftree, &parms, s->lookup_defaults);
> > +    if (errmsg) {
> > +        ap_log_error(APLOG_MARK, APLOG_STARTUP | APLOG_NOERRNO, 0, NULL,
> > +                     "Syntax error on line %d of %s:",
> > +                     parms.err_directive->line_num,
> > +                     parms.err_directive->filename);
> > +        exit(1);
> 
> The errmsg should also be logged (like ap_process_resource_config() does)

I copied the code from there, must have missed a line.  Whoops.  This will
all me fixed and a commit will be made after the alpha.

Ryan

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


Mime
View raw message