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 2001/11/13 03:22:18 UTC
Re: cvs commit: modperl-2.0/xs/tables/current/ModPerl FunctionTable.pm
> #define mpxs_Apache_server(classname) \
> modperl_global_get_server_rec()
> +
> +static MP_INLINE char *mpxs_ap_server_root_relative(pTHX_
> + const char *fname,
> + apr_pool_t *p)
> +{
> + if (!p) {
> + /* XXX: should do something better if called at request time
> + * without a pool
> + */
> + p = modperl_global_get_pconf();
> + }
> + return ap_server_root_relative(p, fname);
> +}
>
ap_server_root_relative return the dir incliding the trailing / if fname==NULL, which is bad.
Any idea why is it so? And whether we can :
char *dir;
...
dir = ap_server_root_relative(p, fname);
if (!fname)
dir[strlen(dir)] = "\0";
}
return dir;
_____________________________________________________________________
Stas Bekman JAm_pH -- Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide http://perl.apache.org/guide
mailto:stas@stason.org http://ticketmaster.com http://apacheweek.com
http://singlesheaven.com http://perl.apache.org http://perlmonth.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: cvs commit: modperl-2.0/xs/tables/current/ModPerl FunctionTable.pm
Posted by Doug MacEachern <do...@covalent.net>.
Apache::server_root_relative is now fully 1.x compat and there is
a new Apache::server_root constant (no trailing /).
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: cvs commit: modperl-2.0/xs/tables/current/ModPerl FunctionTable.pm
Posted by Stas Bekman <st...@stason.org>.
> maybe what we should do is have Apache->server_root_relative that behaves
> just like 1.x in Apache::compat and have 2.0's function be named
> Apache::server_root() which returns ap_server_root if fname == NULL and
> calls ap_server_root_relative() otherwise. this would also fix the
> current compat problem that 2.0's Apache::server_root_relative doesn't
> support Apache->server_root_relative.
then you will write code like:
$conf_dir = Apache::server_root('conf');
does it look good to you? It doesn't to me, since you loose the _relative...
It's easy to change server_root_relative, to optionally (via ref check)
accept a class ref as a first argument. Is there something wrong with it?
And then simply provide server_root() which just returns the root.
_____________________________________________________________________
Stas Bekman JAm_pH -- Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide http://perl.apache.org/guide
mailto:stas@stason.org http://ticketmaster.com http://apacheweek.com
http://singlesheaven.com http://perl.apache.org http://perlmonth.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: cvs commit: modperl-2.0/xs/tables/current/ModPerl FunctionTable.pm
Posted by Doug MacEachern <do...@covalent.net>.
On Thu, 15 Nov 2001, Stas Bekman wrote:
> ok, that sounds good. I say we should change it. My reasoning is that
> the API never defined the case where fname==NULL, so it wasn't a part of
> the public API and users weren't supposed to use this method with
> fname==NULL => we can safely add this to the API.
but Apache->server_root_relative() is a documented part of the 1.x
api. it might not explicitly say 'returns a trailing /' but anybody who's
using it currently is getting a trailing / and might depend on that.
> In any case if some code goes broken, the fix is just to add /.
then you end up with ugly conditional code to make it work with both 1.x
and 2.0
> BTW, we can also expose ap_server_root and then the problem is fixed as
> well.
maybe what we should do is have Apache->server_root_relative that behaves
just like 1.x in Apache::compat and have 2.0's function be named
Apache::server_root() which returns ap_server_root if fname == NULL and
calls ap_server_root_relative() otherwise. this would also fix the
current compat problem that 2.0's Apache::server_root_relative doesn't
support Apache->server_root_relative.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: cvs commit: modperl-2.0/xs/tables/current/ModPerl FunctionTable.pm
Posted by Stas Bekman <st...@stason.org>.
Doug MacEachern wrote:
> On Wed, 14 Nov 2001, Stas Bekman wrote:
>
>
>>I've tested, it has a trailing /
>>
>
> then changing it to not have a trailing / could break people's
> existing code.
>
>
>>Do you think this patch makes sense? or should this be handled on the
>>apr_filepath_merge level?
>>
>
> no, ap_server_root_relative() shouldn't be changed at all, c modules
> already have access to ap_server_root if they need it. we could change
> mpxs_ap_server_root_relative() to return ap_server_root if fname == NULL,
> i just worry about what 1.x code out there that depends on the trailing /
ok, that sounds good. I say we should change it. My reasoning is that
the API never defined the case where fname==NULL, so it wasn't a part of
the public API and users weren't supposed to use this method with
fname==NULL => we can safely add this to the API.
In any case if some code goes broken, the fix is just to add /.
BTW, we can also expose ap_server_root and then the problem is fixed as
well.
_____________________________________________________________________
Stas Bekman JAm_pH -- Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide http://perl.apache.org/guide
mailto:stas@stason.org http://ticketmaster.com http://apacheweek.com
http://singlesheaven.com http://perl.apache.org http://perlmonth.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: cvs commit: modperl-2.0/xs/tables/current/ModPerl FunctionTable.pm
Posted by Doug MacEachern <do...@covalent.net>.
On Wed, 14 Nov 2001, Stas Bekman wrote:
> I've tested, it has a trailing /
then changing it to not have a trailing / could break people's
existing code.
> Do you think this patch makes sense? or should this be handled on the
> apr_filepath_merge level?
no, ap_server_root_relative() shouldn't be changed at all, c modules
already have access to ap_server_root if they need it. we could change
mpxs_ap_server_root_relative() to return ap_server_root if fname == NULL,
i just worry about what 1.x code out there that depends on the trailing /
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: cvs commit: modperl-2.0/xs/tables/current/ModPerl FunctionTable.pm
Posted by Stas Bekman <st...@stason.org>.
Doug MacEachern wrote:
> On Tue, 13 Nov 2001, Stas Bekman wrote:
>
>
>>ap_server_root_relative return the dir incliding the trailing / if
>>fname==NULL, which is bad.
>>
>
> need to do some research here:
> - what does 1.x do? if adds trailing /, then leave it alone
I've tested, it has a trailing /
It's possible that nobody used this before, so the bogosity wasn't noticed.
The header doesn't define the case where fname == NULL:
/**
* For modules which need to read config files, open logs, etc. this
returns
* the canonical form of fname made absolute to ap_server_root.
* @param p pool to allocate data from
* @param fname The file name
*/
AP_DECLARE(char *) ap_server_root_relative(apr_pool_t *p, const char
*fname);
Do you think this patch makes sense? or should this be handled on the
apr_filepath_merge level?
Index: server/config.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/server/config.c,v
retrieving revision 1.136
diff -u -r1.136 config.c
--- server/config.c 2001/10/07 04:54:53 1.136
+++ server/config.c 2001/11/14 08:15:10
@@ -1184,7 +1184,9 @@
AP_DECLARE(char *) ap_server_root_relative(apr_pool_t *p, const char
*file)
{
char *newpath;
- if (apr_filepath_merge(&newpath, ap_server_root, file,
+ if (!file)
+ return ap_server_root;
+ else if (apr_filepath_merge(&newpath, ap_server_root, file,
APR_FILEPATH_TRUENAME, p) == APR_SUCCESS)
return newpath;
else
_____________________________________________________________________
Stas Bekman JAm_pH -- Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide http://perl.apache.org/guide
mailto:stas@stason.org http://ticketmaster.com http://apacheweek.com
http://singlesheaven.com http://perl.apache.org http://perlmonth.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: cvs commit: modperl-2.0/xs/tables/current/ModPerl FunctionTable.pm
Posted by Doug MacEachern <do...@covalent.net>.
On Tue, 13 Nov 2001, Stas Bekman wrote:
> ap_server_root_relative return the dir incliding the trailing / if
> fname==NULL, which is bad.
need to do some research here:
- what does 1.x do? if adds trailing /, then leave it alone
if no trailing /, bug is in apache-2.0
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org