httpd-apreq-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stas Bekman <s...@stason.org>
Subject Re: cvs commit: httpd-apreq-2/glue/perl/xsbuilder/Apache/Upload Apache__Upload.h
Date Fri, 26 Nov 2004 23:20:36 GMT
joes@apache.org wrote:

>   Tag all the *2sv macros with a "parent" argument to ensure the parent object
>   isn't destroyed until the child object is done with it.  Without this patch
>   the following will dump core on the command line, because the underlying pool
>   would be destroyed after the first statement is executed- thus the segfault
>   in param().  This patch fixes that.
>   
>     % perl -MApache2 -MApache::Request -MAPR::Pool
>       -wle '$r=Apache::Request->new(APR::Pool->new); print $r->param("foo")'

I'm looking at this change to adopt for modperl (per the current 
discussion on the modperl dev list). I've some comments below:

>   Revision  Changes    Path
>   1.32      +14 -13    httpd-apreq-2/glue/perl/xsbuilder/apreq_xs_postperl.h
>   
>   Index: apreq_xs_postperl.h
>   ===================================================================
>   RCS file: /home/cvs/httpd-apreq-2/glue/perl/xsbuilder/apreq_xs_postperl.h,v
>   retrieving revision 1.31
>   retrieving revision 1.32
>   diff -u -r1.31 -r1.32
>   --- apreq_xs_postperl.h	1 Jul 2004 18:26:33 -0000	1.31
>   +++ apreq_xs_postperl.h	2 Jul 2004 04:40:08 -0000	1.32
>   @@ -107,7 +107,7 @@
>     * @return Reference to the new Perl object in class.
>     */
>    APR_INLINE
>   -static SV *apreq_xs_c2perl(pTHX_ void *obj, void *env, const char *class)
>   +static SV *apreq_xs_c2perl(pTHX_ void *obj, void *env, const char *class, SV *parent)
>    {
>        SV *rv = sv_setref_pv(newSV(0), class, obj);
>        if (env) {
>   @@ -120,7 +120,7 @@
>             * 5.8.x is OK with the old way as well, but in the future
>             * we may have to use "#if PERL_VERSION < 8" ...
>             */
>   -        sv_magic(SvRV(rv), Nullsv, PERL_MAGIC_ext, Nullch, -1);
>   +        sv_magic(SvRV(rv), parent, PERL_MAGIC_ext, Nullch, -1);
>            SvMAGIC(SvRV(rv))->mg_ptr = env;
>        }
>        return rv;

1) the new param is undocumented (not sure how to explain it, see below)

2) what happens if env == NULL? The parent dependancy will not be created. 
(see the suggested fix below) If my correction is right, should the 
comment be moved up too?

Index: glue/perl/xsbuilder/apreq_xs_postperl.h
===================================================================
--- glue/perl/xsbuilder/apreq_xs_postperl.h     (revision 106667)
+++ glue/perl/xsbuilder/apreq_xs_postperl.h     (working copy)
@@ -111,12 +111,14 @@
   * @param obj C object.
   * @param env C environment.
   * @param class Class perl object will be blessed into.
+ * @param parent XXX
   * @return Reference to the new Perl object in class.
   */
  APR_INLINE
  static SV *apreq_xs_c2perl(pTHX_ void *obj, void *env, const char 
*class, SV *parent)
  {
      SV *rv = sv_setref_pv(newSV(0), class, obj);
+    sv_magic(SvRV(rv), parent, PERL_MAGIC_ext, Nullch, -1);
      if (env) {
          /* We use the old idiom for sv_magic() below,
           * because perl 5.6 mangles the env pointer on
@@ -127,7 +129,6 @@
           * 5.8.x is OK with the old way as well, but in the future
           * we may have to use "#if PERL_VERSION < 8" ...
           */
-        sv_magic(SvRV(rv), parent, PERL_MAGIC_ext, Nullch, -1);
          SvMAGIC(SvRV(rv))->mg_ptr = env;
      }
      return rv;



-- 
__________________________________________________________________
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

Mime
View raw message