ok, here's another pass - I think it covers what we talked about this afternoon on irc.
it won't apply over the is_perl_option_enabled stuff you just posted, so I'll wait until monday to rework it into core and commit, even if there are no technical changes to be made.
if you have any ideas on a better way to phrase Changes, I'm all ears.
Besides comments below, I think we shouldn't support:
Apache->server_root_relative and equivalents, but only:
($r|$s|$c)->server_root_relative(...) and Apache::server_root_relative($pool, ...)
because users will get bad memory leaks when a global pool is used to allocate memory in requests. We should support the non-(pool|$r|$c|$s) argument functions only in compat and may be even print a warning when used for the first time.
What do you think?
Index: Changes
===================================================================
RCS file: /home/cvspublic/modperl-2.0/Changes,v
retrieving revision 1.297
diff -u -r1.297 Changes
--- Changes 3 Jan 2004 01:17:33 -0000 1.297
+++ Changes 10 Jan 2004 02:46:07 -0000
@@ -11,6 +11,8 @@
=over 3
=item 1.99_13-dev
+server_root_relative() will now derive the pool if called from
+$r, $s, or $c objects. [Geoffrey Young]
How about:
server_root_relative() now derives the pool if called from
$r, $s, or $c objects and copies the result so it can be used even if the pool used to allocate the result went out of scope. [Geoffrey Young]
On Solaris add a workaround for xs/APR/APR/Makefile.PL to build
APR.so, correctly linked against apr and apr-util libs, by addding the
Index: src/modules/perl/modperl_util.c
===================================================================
RCS file: /home/cvspublic/modperl-2.0/src/modules/perl/modperl_util.c,v
retrieving revision 1.59
diff -u -r1.59 modperl_util.c
--- src/modules/perl/modperl_util.c 19 Dec 2003 01:17:32 -0000 1.59
+++ src/modules/perl/modperl_util.c 10 Jan 2004 02:46:07 -0000
@@ -194,6 +194,12 @@
return modperl_global_get_pconf();
}
the comments here are confusing. This is a generic internal modperl_sv2pool function and used in other places, so I think only the second para of the comment is relevant. Also it's not Apache->server_root_relative($p, $name), but Apache::server_root_relative($p, $name)
+ /* $r->server_root_relative($name), or
+ * Apache->server_root_relative($p, $name)
+ *
+ * for $(r|c|s) dig out the pool from the record
+ * otherwise use the pool provided
+ */
if ((SvROK(obj) && (SvTYPE(SvRV(obj)) == SVt_PVMG))) {
ptr = SvObjIV(obj);
classname = SvCLASS(obj);
@@ -204,8 +210,13 @@
}
if (*classname != 'A') {
- /* XXX: could be a subclass */
- return NULL;
+ /* XXX: could be a subclass, but more likely
+ * Apache::server_root_relative($name);
+ * note no -> so $name is the class
+ * avoid core dumps by returning the global pool,
+ * even though it then gives just the ServerRoot back
+ */
+ return modperl_global_get_pconf();
}
again, this function is not used solely by s.r.r. returning a global pool could be a very bad guess, causing bad memory leaks. I'd keep that NULL in place and handle the checking for subclass in another iteration, but definitely not returning a global pool.
Index: xs/Apache/ServerUtil/Apache__ServerUtil.h =================================================================== RCS file: /home/cvspublic/modperl-2.0/xs/Apache/ServerUtil/Apache__ServerUtil.h,v retrieving revision 1.8 diff -u -r1.8 Apache__ServerUtil.h --- xs/Apache/ServerUtil/Apache__ServerUtil.h 19 Nov 2001 23:46:48 -0000 1.8 +++ xs/Apache/ServerUtil/Apache__ServerUtil.h 10 Jan 2004 02:46:07 -0000 @@ -42,13 +42,37 @@ #define mpxs_Apache_server(classname) \ modperl_global_get_server_rec()
-static MP_INLINE char *mpxs_ap_server_root_relative(pTHX_
- SV *sv,
- const char *fname)
+static MP_INLINE SV *mpxs_ap_server_root_relative(pTHX_
+ SV *sv,
+ const char *fname)
{
apr_pool_t *p = modperl_sv2pool(aTHX_ sv);
- return ap_server_root_relative(p, fname);
please add a comment of why do you do that, e.g.:
/* make a copy of the result to avoid situations where the pool goes * out of scope and the sv containing a pointer to a freed memory is * attempted to be used */
+ return newSVpv(ap_server_root_relative(p, fname), 0); +}
I don't think the non-Server functions belong here. They should go into respective xs/Apache/ files. Can't #define be used here, they are all the same?
+static MP_INLINE
+SV *mpxs_Apache__RequestRec_server_root_relative(pTHX_
+ SV *sv,
+ const char *fname)
+{
+ return mpxs_ap_server_root_relative(aTHX_ sv, fname);
+}
+
+static MP_INLINE
+SV *mpxs_Apache__Server_server_root_relative(pTHX_
+ SV *sv,
+ const char *fname)
+{
+ return mpxs_ap_server_root_relative(aTHX_ sv, fname);
+}
+
+static MP_INLINE
+SV *mpxs_Apache__Connection_server_root_relative(pTHX_
+ SV *sv,
+ const char *fname)
+{
+ return mpxs_ap_server_root_relative(aTHX_ sv, fname);
}
static void mpxs_Apache__ServerUtil_BOOT(pTHX)
Index: xs/maps/modperl_functions.map
===================================================================
RCS file: /home/cvspublic/modperl-2.0/xs/maps/modperl_functions.map,v
retrieving revision 1.63
diff -u -r1.63 modperl_functions.map
--- xs/maps/modperl_functions.map 23 Dec 2003 03:02:34 -0000 1.63
+++ xs/maps/modperl_functions.map 10 Jan 2004 02:46:07 -0000
@@ -69,8 +69,15 @@
mpxs_Apache__Server_get_handlers
modperl_config_insert_server | | | add_config
+PACKAGE=Apache::RequestRec
+ mpxs_Apache__RequestRec_server_root_relative | | SV *:p, fname=""
+
PACKAGE=Apache::Server
SV *:DEFINE_dir_config | | server_rec *:s, char *:key=NULL, SV *:sv_val=Nullsv
+ mpxs_Apache__Server_server_root_relative | | SV *:p, fname=""
+
+PACKAGE=Apache::Connection
+ mpxs_Apache__Connection_server_root_relative | | SV *:p, fname=""
PACKAGE=Apache
server_rec *:DEFINE_server | | SV *:classname=Nullsv
__________________________________________________________________ Stas Bekman JAm_pH ------> Just Another mod_perl Hacker http://stason.org/ mod_perl Guide ---> http://perl.apache.org mailto:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com http://modperlbook.org http://apache.org http://ticketmaster.com
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
