On Thu, 2003-05-29 at 07:13, Stas Bekman wrote:
Geoffrey Young wrote:
The problem pointed out by Ken is that it would be nice to be able to call
Apache->request($hr);
So that later call to Apache->request (by other modules) would return the subclassed object.
it would not only be nice, request() is documented to behave this way :)
Well, the following patch does just that. Seems fine to me but I'd like to get a few more eyeballs on this one.
nice work philippe :)
gozer++
the only thing that concerns me here is the stability this may take away from the 1.0 tree for very little value - although stuff like Apache::Filter might benefit from this feature, I'm not sure that adding it now is worth it, since any interested modules are becoming deprecated in 2.0.
/me has the same concern. May be keep it in the STATUS file as an available patch. After all there was only one request for this feature. If the patch is available those who want it can try it and if tested enough, can enter 1.29.
Sounds like a good idea to me!
let's go with it then.
regarding the patch. why do you keep both IV and SV? Also looks like you mix usages of NULL and Nullsv for SV* values. Otherwise looks good.
Well, on the first item of having both IV and SV, it's quite simple in an evil way.
My first attempt, after Doug's suggestion in his original comments, was to simply convert mp_request_rec from an IV to a SV.
But there are problems with that approach. Whenever a user gets a $r, either thru the first argument of a handle or thru Apache->request, a new RV is created and blessed on the fly for it. Plus, in more than one place, the value of mp_request_rec is reset by the mp core in case of subrequests and so on.
After some major fiddling, I've found that in 99% of the cases, Apache->request(something) won't be used, so converting a simple IV to a SV was both overkill and a reference count nightmare.
So the current patch simply keeps using the good old IV most of the time, but if and when someone starts assigning it thru Apache->request(), that user-supplied SV gets saved in mp_request_rec_sv and successive calls to Apache->request will return it.
Also worth noting as a side effect, DESTROY will be called on that object after the CleanupHandlers have been run.
+1
As a side note, there are a few places in lib that still call Apache->request($r) for what seems to me like bogus reasons.
Objections to at least cleaning that stuff up prior to 1.28 ?
I'm not sure why that code was there. So I'm not so confident on removing it. mp1's test suite is too poor for regression testing.
Index: lib/Apache/Registry.pm
===================================================================
RCS file: /home/cvs/modperl/lib/Apache/Registry.pm,v
retrieving revision 1.34
diff -u -I$Id -r1.34 Registry.pm
--- lib/Apache/Registry.pm 23 May 2002 04:21:07 -0000 1.34
+++ lib/Apache/Registry.pm 28 May 2003 12:53:54 -0000
@@ -33,13 +33,6 @@
sub handler {
my $r = shift;
- if(ref $r) {
- $r->request($r);
- }
- else {
- #warn "Registry args are: ($r, @_)\n";
- $r = Apache->request;
- }
This has to do with subclasses, I suppose. Which seem to allow calling handler() with no real $r object. I'd keep it in.
Index: lib/Apache/Status.pm
===================================================================
RCS file: /home/cvs/modperl/lib/Apache/Status.pm,v
retrieving revision 1.28
diff -u -I$Id -r1.28 Status.pm
--- lib/Apache/Status.pm 28 Nov 2002 09:42:45 -0000 1.28
+++ lib/Apache/Status.pm 28 May 2003 12:53:54 -0000
@@ -55,7 +55,6 @@
sub handler {
my($r) = @_;
- Apache->request($r); #for Apache::CGI
What's Apache::CGI? If you remove it, will it break that module?
__________________________________________________________________ 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]
