Date: Sun, 16 Feb 1997 01:38:40 -0700 (MST)
From: Marc Slemko <marcs@znep.com>
To: new-httpd@hyperreal.com
Subject: Re: [PATCH] multiple <Directory> matches at same level
In-Reply-To: <Pine.LNX.3.95dg2.970207000109.16924L-100000@twinlark.arctic.org>
Message-ID: <Pine.BSF.3.95.970216013410.23557a-100000@alive.znep.com>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
Sender: new-httpd-owner@apache.org
Precedence: bulk
Reply-To: new-httpd@hyperreal.com

On Fri, 7 Feb 1997, Dean Gaudet wrote:

> On Fri, 7 Feb 1997, Marc Slemko wrote:
> >   * http_request.c: directory_walk() misses some <Directory ...> directives
> >     (PR#131).  I'm not sure the way things are done is really incorrect, 
> >     but should be looked at.
> 
> The complaint is that if two <Directory>s match the same dir (at the same
> level, i.e. <Directory /a/b> and later on another <Directory /a/b>) that
> it ignores the first one.  I posted on this earlier saying that the patch
> supplied works but wondered if we wanted to support it this way.  After
> looking at the code again I noticed that <Location> and <File> already
> behave in this manner (for obvious reasons).  So I'm resubmitting the
> patch with my vote.

This patch was committed in Dean's performance enhancement patches 
(97/02/10 07:17:46).

I don't recall discussion of that.  Was it intentional, or did it
accidently slip in there?

If we are keeping it (the whole reason I noticed it is because I was going
to +1 the patch...) then I will close the PR and commit a spacing change
so things are indented correctly; right now it only lines up right if you
have ts=6.

If not, we should get rid of it.  I would recommend keeping it.

> 
> One issue I had with it was that by doing this we don't allow ourselves to
> implement a "short circuit" optimization in directory_walk().  But I'm now
> of the opinion that directory_walk() is just poorly implemented (it works,
> but it's definately the brute-force method) and that there is significant
> room for other optimization that would eliminate the benefit from such
> a short-circuit.  In particular, the <Directory>/<Location>/<File>
> stuff is static once the config is read, other than .htaccess files.
> It might be possible to redo this using lexical scanning techniques, which
> fall back to .htaccess and brute-force only when the config requires it.
> (I know that high-end servers generally don't require it, 'cause they
> can run AllowOverrides None.) Apache 2.0.
> 
> Dean
> 
> Index: http_request.c
> ===================================================================
> RCS file: /export/home/cvs/apache/src/http_request.c,v
> retrieving revision 1.40
> diff -c -3 -r1.40 http_request.c
> *** http_request.c	1997/01/25 15:44:39	1.40
> --- http_request.c	1997/02/01 21:40:32
> ***************
> *** 370,384 ****
>   	    }
>   	    else if (!strcmp (this_dir, entry_dir))
>   	        this_conf = entry_config;
> - 	}
>   
> ! 	if (this_conf)
> ! 	{
> ! 	    per_dir_defaults =
> ! 	        merge_per_dir_configs (r->pool, per_dir_defaults, this_conf);
> ! 	    core_dir =(core_dir_config *)get_module_config(per_dir_defaults,
>   							   &core_module);
>   	}
>   	overrides_here = core_dir->override;
>   
>   	/* If .htaccess files are enabled, check for one.
> --- 370,385 ----
>   	    }
>   	    else if (!strcmp (this_dir, entry_dir))
>   	        this_conf = entry_config;
>   
> !           if (this_conf)
> !           {
> !               per_dir_defaults =
> !                   merge_per_dir_configs (r->pool, per_dir_defaults, this_conf);
> !               core_dir =(core_dir_config *)get_module_config(per_dir_defaults,
>   							   &core_module);
> +           }
>   	}
> + 
>   	overrides_here = core_dir->override;
>   
>   	/* If .htaccess files are enabled, check for one.
> 
> 
> 
> 
>