You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@perl.apache.org by Stas Bekman <st...@stason.org> on 2004/07/03 02:59:22 UTC
[mp2] modperl_config_insert_request's path argument
Philippe, I've noticed this comment http_config.h:
struct cmd_parms_struct {
[...]
/** If configuring for a directory, pathname of that directory.
* NOPE! That's what it meant previous to the existance of <Files>,
* <Location> and regex matching. Now the only usefulness that can be
* derived from this field is whether a command is being called in a
* server context (path == NULL) or being called in a dir context
* (path != NULL). */
char *path;
Which I think makes the optional 2nd arg 'path' to s->add_config redundant.
Moreover the default (r->filename) might be incorrect if it's NULL. So should
we drop it completely in here:
const char *modperl_config_insert_request(pTHX_
request_rec *r,
SV *lines,
char *path,
int override)
{
const char *errmsg;
ap_conf_vector_t *dconf = ap_create_per_dir_config(r->pool);
errmsg = modperl_config_insert(aTHX_
r->server, r->pool, r->pool,
override, path,
dconf, lines);
replacing it with '/' or something? Or am I misreading it? We don't have any
tests for this and I haven't looked at config stuff for ages, so I'm not sure
whether we should keep it or drop it.
What do you think?
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [Patch mp2] deprecate modperl_config_insert_request's path argument
Posted by Stas Bekman <st...@stason.org>.
Philippe M. Chiasson wrote:
[...]
>>> I've looked at it just now, and I think you are correct. I can't seem
>>> to find a reason why we should be doing this. And you are also
>>> correcet in saying that a directive for a null r->filename might be
>>> treated as a server configuration accidentally.
>>>
>>>
>>>> We don't have any tests for this and I haven't looked at config
>>>> stuff for ages, so I'm not sure whether we should keep it or drop it.
>>>
>>>
>>>
>>>> What do you think?
>>>
>>>
>>> I think it's a good idea to get rid of it somehow like this :
>>
>>
>> +1!
Perfect, +1!
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
[Patch mp2] deprecate modperl_config_insert_request's path argument
Posted by "Philippe M. Chiasson" <go...@ectoplasm.org>.
Stas Bekman wrote:
> Philippe M. Chiasson wrote:
>
>>Stas Bekman wrote:
>>
>>
>>>Philippe, I've noticed this comment http_config.h:
>>>
>>>struct cmd_parms_struct {
>>>[...]
>>> /** If configuring for a directory, pathname of that directory.
>>> * NOPE! That's what it meant previous to the existance of <Files>,
>>> * <Location> and regex matching. Now the only usefulness that
>>>can be
>>> * derived from this field is whether a command is being called in a
>>> * server context (path == NULL) or being called in a dir context
>>> * (path != NULL). */
>>> char *path;
>>>
>>>Which I think makes the optional 2nd arg 'path' to s->add_config
>>>redundant. Moreover the default (r->filename) might be incorrect if
>>>it's NULL. So should we drop it completely in here:
>>>
>>>const char *modperl_config_insert_request(pTHX_
>>> request_rec *r,
>>> SV *lines,
>>> char *path,
>>> int override)
>>>{
>>> const char *errmsg;
>>> ap_conf_vector_t *dconf = ap_create_per_dir_config(r->pool);
>>>
>>> errmsg = modperl_config_insert(aTHX_
>>> r->server, r->pool, r->pool,
>>> override, path,
>>> dconf, lines);
>>>
>>>replacing it with '/' or something? Or am I misreading it?
>>
>>
>>I've looked at it just now, and I think you are correct. I can't seem to
>>find a reason why we should be doing this. And you are also
>>correcet in saying that a directive for a null r->filename might be
>>treated as a server configuration accidentally.
>>
>>
>>>We don't have any tests for this and I haven't looked at config stuff
>>>for ages, so I'm not sure whether we should keep it or drop it.
>>
>>
>>>What do you think?
>>
>>I think it's a good idea to get rid of it somehow like this :
>
> +1!
>
Index: Changes
===================================================================
RCS file: /home/cvs/modperl-2.0/Changes,v
retrieving revision 1.402
diff -u -I$Id -r1.402 Changes
--- Changes 5 Jul 2004 22:02:42 -0000 1.402
+++ Changes 6 Jul 2004 20:31:52 -0000
@@ -12,6 +12,8 @@
=item 1.99_15-dev
+Removed the deprecated path argument to $r->add_config() [Gozer]
+
Created a META.yml for CPAN and friends, including Apache-Test as
a private resource to keep CPAN from installing mod_perl when a
user just wants Apache::Test [Gozer]
Index: docs/api/Apache/RequestUtil.pod
===================================================================
RCS file: /home/cvs/modperl-docs/src/docs/2.0/api/Apache/RequestUtil.pod,v
retrieving revision 1.20
diff -u -I$Id -r1.20 RequestUtil.pod
--- docs/api/Apache/RequestUtil.pod 5 Jul 2004 04:37:20 -0000 1.20
+++ docs/api/Apache/RequestUtil.pod 6 Jul 2004 20:31:52 -0000
@@ -101,8 +101,7 @@
Dynamically add Apache configuration at request processing runtime:
$r->add_config($lines);
- $r->add_config($lines, $path);
- $r->add_config($lines, $path, $override);
+ $r->add_config($lines, $override);
=over 4
@@ -114,12 +113,7 @@
An ARRAY reference containing configuration lines per element, without
the new line terminators.
-=item opt arg2: C<$path> ( SCALAR )
-
-This argument is only useful to tell whether a command is being called
-in a dir context. By default C<$r-E<gt>filename> is passed.
-
-=item opt arg3: C<$override> ( C<L<APR::Const status
+=item opt arg2: C<$override> ( C<L<APR::Const status
constant|docs::2.0::api::APR::Const>> )
Which allow-override bits are set
Index: src/modules/perl/modperl_config.c
===================================================================
RCS file: /home/cvs/modperl-2.0/src/modules/perl/modperl_config.c,v
retrieving revision 1.79
diff -u -I$Id -r1.79 modperl_config.c
--- src/modules/perl/modperl_config.c 16 Jun 2004 03:55:47 -0000 1.79
+++ src/modules/perl/modperl_config.c 6 Jul 2004 20:31:53 -0000
@@ -573,15 +573,16 @@
const char *modperl_config_insert_request(pTHX_
request_rec *r,
SV *lines,
- char *path,
int override)
{
const char *errmsg;
ap_conf_vector_t *dconf = ap_create_per_dir_config(r->pool);
+ /* The path argument of "/" is only required to be non-NULL
+ and "/" is as good a default as anything else */
errmsg = modperl_config_insert(aTHX_
r->server, r->pool, r->pool,
- override, path,
+ override, "/",
dconf, lines);
if (errmsg) {
Index: src/modules/perl/modperl_config.h
===================================================================
RCS file: /home/cvs/modperl-2.0/src/modules/perl/modperl_config.h,v
retrieving revision 1.33
diff -u -I$Id -r1.33 modperl_config.h
--- src/modules/perl/modperl_config.h 4 Mar 2004 06:01:07 -0000 1.33
+++ src/modules/perl/modperl_config.h 6 Jul 2004 20:31:53 -0000
@@ -135,7 +135,6 @@
const char *modperl_config_insert_request(pTHX_
request_rec *r,
SV *lines,
- char *path,
int override);
int modperl_config_is_perl_option_enabled(pTHX_ request_rec *r,
Index: xs/Apache/Access/Apache__Access.h
===================================================================
RCS file: /home/cvs/modperl-2.0/xs/Apache/Access/Apache__Access.h,v
retrieving revision 1.8
diff -u -I$Id -r1.8 Apache__Access.h
--- xs/Apache/Access/Apache__Access.h 4 Mar 2004 06:01:11 -0000 1.8
+++ xs/Apache/Access/Apache__Access.h 6 Jul 2004 20:31:55 -0000
@@ -80,7 +80,7 @@
errmsg =
modperl_config_insert_request(aTHX_ r,
newRV_noinc((SV*)config),
- r->filename, OR_AUTHCFG);
+ OR_AUTHCFG);
if (errmsg) {
Perl_warn(aTHX_ "Can't change %s to '%s'\n", directive, val);
Index: xs/Apache/RequestUtil/Apache__RequestUtil.h
===================================================================
RCS file: /home/cvs/modperl-2.0/xs/Apache/RequestUtil/Apache__RequestUtil.h,v
retrieving revision 1.22
diff -u -I$Id -r1.22 Apache__RequestUtil.h
--- xs/Apache/RequestUtil/Apache__RequestUtil.h 4 Mar 2004 06:01:13 -0000 1.22
+++ xs/Apache/RequestUtil/Apache__RequestUtil.h 6 Jul 2004 20:31:55 -0000
@@ -271,10 +271,10 @@
}
static MP_INLINE
-void mpxs_Apache__RequestRec_add_config(pTHX_ request_rec *r, SV *lines, char *path, int override)
+void mpxs_Apache__RequestRec_add_config(pTHX_ request_rec *r, SV *lines, int override)
{
const char *errmsg = modperl_config_insert_request(aTHX_ r, lines,
- path, override);
+ override);
if (errmsg) {
Perl_croak(aTHX_ "$r->add_config() has failed: %s", errmsg);
}
Index: xs/tables/current/ModPerl/FunctionTable.pm
===================================================================
RCS file: /home/cvs/modperl-2.0/xs/tables/current/ModPerl/FunctionTable.pm,v
retrieving revision 1.167
diff -u -I$Id -r1.167 FunctionTable.pm
--- xs/tables/current/ModPerl/FunctionTable.pm 2 Jul 2004 22:43:17 -0000 1.167
+++ xs/tables/current/ModPerl/FunctionTable.pm 6 Jul 2004 20:31:56 -0000
@@ -6151,10 +6151,6 @@
'name' => 'lines'
},
{
- 'type' => 'char *',
- 'name' => 'path'
- },
- {
'type' => 'int',
'name' => 'override'
}
--
--------------------------------------------------------------------------------
Philippe M. Chiasson m/gozer\@(apache|cpan|ectoplasm)\.org/ GPG KeyID : 88C3A5A5
http://gozer.ectoplasm.org/ F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3A5A5
Re: [mp2] modperl_config_insert_request's path argument
Posted by Stas Bekman <st...@stason.org>.
Philippe M. Chiasson wrote:
> Stas Bekman wrote:
>
>> Philippe, I've noticed this comment http_config.h:
>>
>> struct cmd_parms_struct {
>> [...]
>> /** If configuring for a directory, pathname of that directory.
>> * NOPE! That's what it meant previous to the existance of <Files>,
>> * <Location> and regex matching. Now the only usefulness that
>> can be
>> * derived from this field is whether a command is being called in a
>> * server context (path == NULL) or being called in a dir context
>> * (path != NULL). */
>> char *path;
>>
>> Which I think makes the optional 2nd arg 'path' to s->add_config
>> redundant. Moreover the default (r->filename) might be incorrect if
>> it's NULL. So should we drop it completely in here:
>>
>> const char *modperl_config_insert_request(pTHX_
>> request_rec *r,
>> SV *lines,
>> char *path,
>> int override)
>> {
>> const char *errmsg;
>> ap_conf_vector_t *dconf = ap_create_per_dir_config(r->pool);
>>
>> errmsg = modperl_config_insert(aTHX_
>> r->server, r->pool, r->pool,
>> override, path,
>> dconf, lines);
>>
>> replacing it with '/' or something? Or am I misreading it?
>
>
> I've looked at it just now, and I think you are correct. I can't seem to
> find a reason why we should be doing this. And you are also
> correcet in saying that a directive for a null r->filename might be
> treated as a server configuration accidentally.
>
>> We don't have any tests for this and I haven't looked at config stuff
>> for ages, so I'm not sure whether we should keep it or drop it.
>
>
>> What do you think?
>
>
>
> I think it's a good idea to get rid of it somehow like this :
+1!
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2] modperl_config_insert_request's path argument
Posted by "Philippe M. Chiasson" <go...@ectoplasm.org>.
Stas Bekman wrote:
> Philippe, I've noticed this comment http_config.h:
>
> struct cmd_parms_struct {
> [...]
> /** If configuring for a directory, pathname of that directory.
> * NOPE! That's what it meant previous to the existance of <Files>,
> * <Location> and regex matching. Now the only usefulness that
> can be
> * derived from this field is whether a command is being called in a
> * server context (path == NULL) or being called in a dir context
> * (path != NULL). */
> char *path;
>
> Which I think makes the optional 2nd arg 'path' to s->add_config
> redundant. Moreover the default (r->filename) might be incorrect if
> it's NULL. So should we drop it completely in here:
>
> const char *modperl_config_insert_request(pTHX_
> request_rec *r,
> SV *lines,
> char *path,
> int override)
> {
> const char *errmsg;
> ap_conf_vector_t *dconf = ap_create_per_dir_config(r->pool);
>
> errmsg = modperl_config_insert(aTHX_
> r->server, r->pool, r->pool,
> override, path,
> dconf, lines);
>
> replacing it with '/' or something? Or am I misreading it?
I've looked at it just now, and I think you are correct. I can't seem to
find a reason why we should be doing this. And you are also
correcet in saying that a directive for a null r->filename might be
treated as a server configuration accidentally.
> We don't have any tests for this and I haven't looked at config stuff
> for ages, so I'm not sure whether we should keep it or drop it.
> What do you think?
I think it's a good idea to get rid of it somehow like this :
Index: src/modules/perl/modperl_config.c
===================================================================
RCS file: /home/cvs/modperl-2.0/src/modules/perl/modperl_config.c,v
retrieving revision 1.79
diff -u -I$Id -r1.79 modperl_config.c
--- src/modules/perl/modperl_config.c 16 Jun 2004 03:55:47 -0000 1.79
+++ src/modules/perl/modperl_config.c 5 Jul 2004 20:31:17 -0000
@@ -573,7 +573,6 @@
const char *modperl_config_insert_request(pTHX_
request_rec *r,
SV *lines,
- char *path,
int override)
{
const char *errmsg;
@@ -581,7 +580,7 @@
errmsg = modperl_config_insert(aTHX_
r->server, r->pool, r->pool,
- override, path,
+ override, "/",
dconf, lines);
if (errmsg) {
Index: src/modules/perl/modperl_config.h
===================================================================
RCS file: /home/cvs/modperl-2.0/src/modules/perl/modperl_config.h,v
retrieving revision 1.33
diff -u -I$Id -r1.33 modperl_config.h
--- src/modules/perl/modperl_config.h 4 Mar 2004 06:01:07 -0000 1.33
+++ src/modules/perl/modperl_config.h 5 Jul 2004 20:31:17 -0000
@@ -135,7 +135,6 @@
const char *modperl_config_insert_request(pTHX_
request_rec *r,
SV *lines,
- char *path,
int override);
int modperl_config_is_perl_option_enabled(pTHX_ request_rec *r,
Index: xs/Apache/Access/Apache__Access.h
===================================================================
RCS file: /home/cvs/modperl-2.0/xs/Apache/Access/Apache__Access.h,v
retrieving revision 1.8
diff -u -I$Id -r1.8 Apache__Access.h
--- xs/Apache/Access/Apache__Access.h 4 Mar 2004 06:01:11 -0000 1.8
+++ xs/Apache/Access/Apache__Access.h 5 Jul 2004 20:31:18 -0000
@@ -80,7 +80,7 @@
errmsg =
modperl_config_insert_request(aTHX_ r,
newRV_noinc((SV*)config),
- r->filename, OR_AUTHCFG);
+ OR_AUTHCFG);
if (errmsg) {
Perl_warn(aTHX_ "Can't change %s to '%s'\n", directive, val);
Index: xs/Apache/RequestUtil/Apache__RequestUtil.h
===================================================================
RCS file:
/home/cvs/modperl-2.0/xs/Apache/RequestUtil/Apache__RequestUtil.h,v
retrieving revision 1.22
diff -u -I$Id -r1.22 Apache__RequestUtil.h
--- xs/Apache/RequestUtil/Apache__RequestUtil.h 4 Mar 2004 06:01:13
-0000 1.22
+++ xs/Apache/RequestUtil/Apache__RequestUtil.h 5 Jul 2004 20:31:18 -0000
@@ -271,10 +271,10 @@
}
static MP_INLINE
-void mpxs_Apache__RequestRec_add_config(pTHX_ request_rec *r, SV
*lines, char *path, int override)
+void mpxs_Apache__RequestRec_add_config(pTHX_ request_rec *r, SV
*lines, int override)
{
const char *errmsg = modperl_config_insert_request(aTHX_ r, lines,
- path, override);
+ override);
if (errmsg) {
Perl_croak(aTHX_ "$r->add_config() has failed: %s", errmsg);
}
Index: xs/tables/current/ModPerl/FunctionTable.pm
===================================================================
RCS file: /home/cvs/modperl-2.0/xs/tables/current/ModPerl/FunctionTable.pm,v
retrieving revision 1.167
diff -u -I$Id -r1.167 FunctionTable.pm
--- xs/tables/current/ModPerl/FunctionTable.pm 2 Jul 2004 22:43:17
-0000 1.167
+++ xs/tables/current/ModPerl/FunctionTable.pm 5 Jul 2004 20:31:18 -0000
@@ -6151,10 +6151,6 @@
'name' => 'lines'
},
{
- 'type' => 'char *',
- 'name' => 'path'
- },
- {
'type' => 'int',
'name' => 'override'
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org