>
> Not only not intuitive, but wrong. Consider this case: some function
> returns the wanted amount of chars to slurp, let's say it decides that
> it doesn't want any more chars and sets $wanted to 0 and you passed it
> to $bb->flatten($pool, $wanted); now instead of getting an empty string,
> you get the whole brigade. You could check that $wanted is 0 and not
> call flatten, but hey why making it more error-prone to users.
yes, you're right
>> + return newSVpvn(buffer, length);
>
>
> this is a bad idea. You should install the buffer into the SV not copy it.
ok, both of these issues are addressed in the attached patch.
--Geoff
Index: t/response/TestAPR/flatten.pm
===================================================================
RCS file: /home/cvspublic/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 19:07:00 -0000
@@ -16,7 +16,7 @@
my $r = shift;
- plan $r, tests => 14;
+ plan $r, tests => 16;
# first, create a brigade
my $pool = $r->pool;
@@ -36,10 +36,24 @@
$bb->length,
'APR::Brigade::length()');
+ # syntax: require a $bb
+ eval { APR::Brigade::flatten("", "") };
+
+ ok t_cmp(qr!expecting an APR::Brigade derived object!,
+ $@,
+ 'APR::Brigade::flatten() requires a brigade');
+
+ # syntax: always require a pool
+ eval { $bb->flatten("") };
+
+ ok t_cmp(qr!expecting an APR::Pool derived object!,
+ $@,
+ 'APR::Brigade::flatten() requires a pool');
+
# syntax: always require a pool
eval { $bb->flatten() };
- ok t_cmp(qr/Usage: APR::Brigade::flatten/,
+ ok t_cmp(qr!usage: \$bb->flatten!,
$@,
'APR::Brigade::flatten() requires a pool');
@@ -50,24 +64,24 @@
ok t_cmp(200000,
length($data),
- 'APR::Brigade::flatten() returned all the data');
+ '$bb->flatten($pool) 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($p, 0) returns 0 bytes
{
my $data = $bb->flatten($pool, 0);
- ok t_cmp(200000,
+ t_debug('$bb->flatten($pool, 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($pool, 0) returned no data');
}
@@ -79,7 +93,7 @@
ok t_cmp(30,
length($data),
- 'APR::Brigade::flatten() returned all the data');
+ '$bb->flatten($pool, 30) returned 30 characters');
t_debug("APR::Brigade::flatten() data all 'x' characters");
ok ($data !~ m/[^x]/);
@@ -91,9 +105,9 @@
ok t_cmp(190000,
length($data),
- 'APR::Brigade::flatten() returned all the data');
+ '$bb->flatten($pool, 190000) returned 19000 characters');
- t_debug("APR::Brigade::flatten() data all 'x' characters");
+ t_debug("data all 'x' characters");
ok ($data !~ m/[^x]/);
}
@@ -103,9 +117,9 @@
ok t_cmp(200000,
length($data),
- 'APR::Brigade::flatten() returned all the data');
+ '$bb->flatten($pool, 300000) returned all 200000 characters');
- t_debug("APR::Brigade::flatten() data all 'x' characters");
+ t_debug("data all 'x' characters");
ok ($data !~ m/[^x]/);
}
@@ -113,12 +127,12 @@
{
my $data = APR::Brigade->new($pool, $ba)->flatten($pool);
- 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/cvspublic/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 19:07:00 -0000
@@ -97,28 +97,36 @@
}
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)
{
+ apr_bucket_brigade *bb;
+ apr_pool_t *pool;
+ apr_size_t length;
+ apr_status_t status;
+ char *buffer;
+ SV *data = newSV(0);
+
/* 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_size_t length;
+ if (items < 2) {
+ Perl_croak(aTHX_ "usage: $bb->flatten($pool, [$length])");
+ }
+
+ bb = mp_xs_sv2_APR__Brigade(*MARK);
+ pool = mp_xs_sv2_APR__Pool(*(MARK+1));
- if (SvTRUE(sv_len)) {
+ if (items > 2) {
/* 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);
+ length = SvIV(*(MARK+2));
/* since we always require a pool, we can allocate from it */
buffer = apr_pcalloc(pool, length);
@@ -130,7 +138,7 @@
/* 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
+ * note that length is output only
*/
status = apr_brigade_pflatten(bb, &buffer, &length, pool);
@@ -145,5 +153,12 @@
return &PL_sv_undef;
}
- return newSVpvn(buffer, length);
+ mpxs_sv_grow(data, length);
+ SvPVX(data) = buffer;
+ mpxs_sv_cur_set(data, length);
+
+ /* the brigade could be anything, so taint it */
+ SvTAINTED_on(data);
+
+ return data;
}
Index: xs/maps/apr_functions.map
===================================================================
RCS file: /home/cvspublic/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 19:07:00 -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
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
