Doh! since my suggested change drops the use of apr_brigade_pflatten, all this magic is no longer needed. just use the grow macro like all other functions do and pass SvPVX to flatter. and whoah, no longer we need the pool object! that's a goodness.

Here's the reworked patch (against cvs):

Index: t/response/TestAPR/flatten.pm
===================================================================
RCS file: /home/cvs/modperl-2.0/t/response/TestAPR/flatten.pm,v
retrieving revision 1.2
diff -u -r1.2 flatten.pm
--- t/response/TestAPR/flatten.pm       27 Jan 2004 16:34:36 -0000      1.2
+++ t/response/TestAPR/flatten.pm       28 Jan 2004 23:22:06 -0000
@@ -36,50 +36,50 @@
              $bb->length,
              'APR::Brigade::length()');

-    # syntax: always require a pool
-    eval { $bb->flatten() };
+    # syntax: require a $bb
+    eval { APR::Brigade::flatten("") };

-    ok t_cmp(qr/Usage: APR::Brigade::flatten/,
+    ok t_cmp(qr!expecting an APR::Brigade derived object!,
              $@,
-             'APR::Brigade::flatten() requires a pool');
+             'APR::Brigade::flatten() requires a brigade');

-    # flatten($pool) will slurp up the entire brigade
+    # flatten() will slurp up the entire brigade
     # equivalent to calling apr_brigade_pflatten
     {
-        my $data = $bb->flatten($pool);
+        my $data = $bb->flatten();

         ok t_cmp(200000,
                  length($data),
-                 'APR::Brigade::flatten() returned all the data');
+                 '$bb->flatten() returned all the data');

         # don't use t_cmp() here, else we get 200,000 characters
         # to look at in verbose mode
-        t_debug("APR::Brigade::flatten() data all 'x' characters");
+        t_debug("data all 'x' characters");
         ok ($data !~ m/[^x]/);
     }

-    # syntax: flatten($p, 0) is equivalent to flatten($p)
+    # flatten(0) returns 0 bytes
     {
-        my $data = $bb->flatten($pool, 0);
+        my $data = $bb->flatten(0);

-        ok t_cmp(200000,
+        t_debug('$bb->flatten(0) returns a defined value');
+        ok (defined $data);
+
+        ok t_cmp(0,
                  length($data),
-                 'APR::Brigade::flatten() returned all the data');
-
-        t_debug("APR::Brigade::flatten() data all 'x' characters");
-        ok ($data !~ m/[^x]/);
+                 '$bb->flatten(0) returned no data');
     }


- # flatten($pool, $length) will return the first $length bytes + # flatten($length) will return the first $length bytes # equivalent to calling apr_brigade_flatten { # small - my $data = $bb->flatten($pool, 30); + my $data = $bb->flatten(30);

         ok t_cmp(30,
                  length($data),
-                 'APR::Brigade::flatten() returned all the data');
+                 '$bb->flatten(30) returned 30 characters');

         t_debug("APR::Brigade::flatten() data all 'x' characters");
         ok ($data !~ m/[^x]/);
@@ -87,38 +87,38 @@

     {
         # large
-        my $data = $bb->flatten($pool, 190000);
+        my $data = $bb->flatten(190000);

         ok t_cmp(190000,
                  length($data),
-                 'APR::Brigade::flatten() returned all the data');
+                 '$bb->flatten(190000) returned 19000 characters');

-        t_debug("APR::Brigade::flatten() data all 'x' characters");
+        t_debug("data all 'x' characters");
         ok ($data !~ m/[^x]/);
     }

     {
         # more than enough
-        my $data = $bb->flatten($pool, 300000);
+        my $data = $bb->flatten(300000);

         ok t_cmp(200000,
                  length($data),
-                 'APR::Brigade::flatten() returned all the data');
+                 '$bb->flatten(300000) returned all 200000 characters');

-        t_debug("APR::Brigade::flatten() data all 'x' characters");
+        t_debug("data all 'x' characters");
         ok ($data !~ m/[^x]/);
     }

     # fetch from a brigade with no data in it
     {
-        my $data = APR::Brigade->new($pool, $ba)->flatten($pool);
+        my $data = APR::Brigade->new($pool, $ba)->flatten();

-        t_debug('an empty brigade returns a defined value');
+        t_debug('empty brigade returns a defined value');
         ok (defined $data);

         ok t_cmp(0,
                  length($data),
-                 'an empty brigade returns data of 0 length');
+                 'empty brigade returns data of 0 length');
     }

     Apache::OK;
Index: xs/APR/Brigade/APR__Brigade.h
===================================================================
RCS file: /home/cvs/modperl-2.0/xs/APR/Brigade/APR__Brigade.h,v
retrieving revision 1.7
diff -u -r1.7 APR__Brigade.h
--- xs/APR/Brigade/APR__Brigade.h       28 Jan 2004 13:54:27 -0000      1.7
+++ xs/APR/Brigade/APR__Brigade.h       28 Jan 2004 23:22:06 -0000
@@ -97,46 +97,39 @@
 }

 static MP_INLINE
-SV *mpxs_APR__Brigade_flatten(pTHX_ apr_bucket_brigade *bb,
-                              apr_pool_t *pool, SV *sv_len)
+SV *mpxs_APR__Brigade_flatten(pTHX_ I32 items,
+                              SV **MARK, SV **SP)
 {

-    /* XXX we're deviating from the API here to try and make
-     * the API more Perlish - nobody likes the idea of two
-     * "in/out" arguments.  and we generally don't ever need
-     * the length anyway...
-     */
-
-    apr_status_t status;
-    char *buffer;
+    apr_bucket_brigade *bb;
     apr_size_t length;
+    apr_status_t status;
+    SV *data = newSV(0);

-    if (SvTRUE(sv_len)) {
-        /* APR::Brigade->flatten($p, $length);
-         * use apr_brigade_flatten to get the first $length bytes
-         *
-         * note that $length must be non-zero to get here
-         */
-
-        length = mp_xs_sv2_apr_size_t(sv_len);
-
-        /* since we always require a pool, we can allocate from it */
-        buffer = apr_pcalloc(pool, length);
-
-        status = apr_brigade_flatten(bb, buffer, &length);
+    if (items < 1) {
+        Perl_croak(aTHX_ "usage: $bb->flatten([$length])");
+    }
+
+    bb = mp_xs_sv2_APR__Brigade(*MARK);

+    if (items > 1) {
+        /* APR::Brigade->flatten($p, $length); */
+        length = SvIV(*(MARK+1));
     }
     else {
-        /* APR::Brigade->flatten($p);
-         * use apr_brigade_pflatten to slurp the entire brigade
-         *
-         * note that it doesn't matter what we pass in for length
+        /* APR::Brigade->flatten($p); */
+        /* can't use pflatten, because we can't realloc() memory
+         * allocated by pflatten. and we need to append '\0' to it in
+         * SV.  so we copy pflatten's guts here.
          */
-
-        status = apr_brigade_pflatten(bb, &buffer, &length, pool);
-
+        apr_off_t actual;
+        apr_brigade_length(bb, 1, &actual);
+        length = (apr_size_t)actual;
     }

+    mpxs_sv_grow(data, length);
+
+    status = apr_brigade_flatten(bb, SvPVX(data), &length);
     if (status != APR_SUCCESS) {
         /* XXX croak?
          * note that reading from an empty brigade will return
@@ -145,5 +138,8 @@
         return &PL_sv_undef;
     }

-    return newSVpvn(buffer, length);
+    mpxs_sv_cur_set(data, length);
+    SvTAINTED_on(data);
+
+    return data;
 }
Index: xs/maps/apr_functions.map
===================================================================
RCS file: /home/cvs/modperl-2.0/xs/maps/apr_functions.map,v
retrieving revision 1.68
diff -u -r1.68 apr_functions.map
--- xs/maps/apr_functions.map   28 Jan 2004 13:54:35 -0000      1.68
+++ xs/maps/apr_functions.map   28 Jan 2004 23:22:06 -0000
@@ -98,7 +98,7 @@
  mpxs_APR__Brigade_concat       #APR_BRIGADE_CONCAT
  mpxs_APR__Brigade_empty        #APR_BRIGADE_EMPTY
  mpxs_APR__Brigade_length | | bb, read_all=1
- mpxs_APR__Brigade_flatten | | bb, pool, sv_len=0
+ mpxs_APR__Brigade_flatten | | ...
  mpxs_APR__Brigade_pool

MODULE=APR::Bucket



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



Reply via email to