perl-modperl mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Torsten Foertsch <torsten.foert...@gmx.net>
Subject Re: what is the best way to do ...
Date Fri, 24 Feb 2006 11:50:10 GMT
On Thursday 23 February 2006 23:41, Philippe M. Chiasson wrote:
> It used to pass in NULL all the time, until someone pointed out that it was
> causing a bug with certain directives. So the fix was adding the $path
> argument to add_config().

That someone was me. And it used to pass "/" as path. I already saw the 
additional path parameter in the docs on http://perl.apache.org but the 
current version 2.0.2 does not contain the patch.

The new patch combines 2 things:

1) passing an empty string as path parameter sets the cmd_parms-path to NULL. 
Before you could only set a path different to the default "/". With this new 
feature you can for example

  $r->add_config( ['<Directory /...>',
                   'AllowOverride Options AuthConfig',
                   '</Directory>'],
                  -1, '' );

or

  $r->add_config( ['ProxyPassReverse /path http://proxy/path'], -1, '' );


2) it offers a read-only interface to the new override_opts member of 
cmd_parms, adds $r->allow_override_opts to read the override_opts member of 
core_config and adds a 4th parameter $override_opts to add_config.

> > I  have tried passing also NULL pointers. For some configuration
> > directives that causes segfaults but sometimes it's useful (for example
> > if a <Location> block is to be passed to $r->add_config). I'd like to
> > extend the interface to pass a NULL pointer in cmd_parms if an empty
> > string is passed to add_config.
>
> That's not what's hapenning currently ?

see above. The current implementation throws an error if for example a 
directory or location block is passed. Apache2::ServerUtil::add_config 
supports it but Apache2::RequestUtil::add_config does not.

> > Would such a patch have any chances to get into mod_perl?
>
> Yes, it sounds like a good idea overall. I would suggest you try and break
> up the problem in many smaller patches instead of one big patch. It's
> easier to review, and it usually simplifies the problem being solved at the
> same time.

I rather like a "big" one because it's not so big. The additional NULL path is 
just a (*path ? ... : NULL) at C-level. The other stuff handles 
override_opts, docs and tests.

I think I'll submit the patch later today.

> As for the extra argument to add_config, I thought you mentionned a more
> elegant solution that didn't add an extra argument, but introduced a new
> object wrapper for override_opts that could be passed in. I am curious
> about what hapenned to that idea, it sounded good to me.

I have looked into it for a while. Then I thought a programmer who wants to 
use it has to understand the whole cmd_parms stuff from Apache. Then, it 
would require a lot more work to do to get it running. I was not sure in the 
first place if it was a good idea. Also, I got no reply to that suggestion. 
So, your additional parameter became more and more charming.

But, even with a 4th parameter to add_config the CmdParms solution is still 
feasible. Maybe I get down to it when Apache 2.4 adds another feature.

Torsten

Mime
View raw message