Re: apacheconfig patch review requested

[Daniel Jacobowitz <dan@debian.org>]

On Tue, Jan 15, 2002 at 02:49:11PM -0500, Steve Stock wrote:
> On Tue, Jan 15, 2002 at 01:35:19PM -0500, Daniel Jacobowitz wrote:
> > On Tue, Jan 15, 2002 at 05:27:44PM +0000, Matthew Wilcox wrote:
> > > 
> > > Hi.  Anyone know perl?  :-)
> > > 
> > > Bug #12094 (apache's oldest still-open bug!), #59998, #105820 and now
> > > #129372 would seem to all be fixable by the patch attached to #105820.
> > > Thanks to Steve Stock for supplying the patch.  Since I don't grok perl,
> > > I'd just like someone to review it and see if it looks reasonable.
> > > If it is, I'll incorporate it in apache 1.3.22-6.
> > 
> > Well, the two BUG comments upset me a bit.  The latter at least is a
> > problem.  It should be possible to solve both fairly easily - the
> > former with a hash table (preferably hashing on inode/dev rather than
> > filename) and the latter by prepending /etc/apache to relative paths. 
> > I'm fairly certain Apache does nothing more complicated than that.
> 
> The inode hash is a good idea, looks to be a simple addition.  Any
> thoughts on using a $main::something style variable name?  These appear
> in other parts of the program, but it seems a bit too much like bad
> style to me.
> 
> Relative paths are based off the ServerRoot setting.  By default this
> is set in httpd.conf to /etc/apache, however it may also be set on the
> command line and removed from httpd.conf.  Because I don't know how
> flexible apacheconfig needs to be with respect to local configuration
> changes I left this functionality out and labeled it a bug.  If it is
> safe to assume that ServerRoot will always be set in httpd.conf then
> fixing relative includes is easy enough.

I'd say assuming it was /etc/apache is safe.  You're dealing with the
configuration in /etc/apache, remember?  It's POSSIBLE (but exceedingly
perverse) that it would be set differently.  Parsing for it as we go
along is probably too much work to bother.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer