ok, both of these issues are addressed in the attached patch.
looks good, a few efficiency fixes are due
pcalloc shouldn't be used, unless you really need to 0 all the bytes. palloc is faster.
- return newSVpvn(buffer, length);
+ mpxs_sv_grow(data, length); + SvPVX(data) = buffer;
+ mpxs_sv_cur_set(data, length);
but this is the same as newSVpvn(buffer, length);. You still allocate the buffer twice (second time in mpxs_sv_grow which calls SvGROW). I've attached the adjusted patch (of only this file), which allocates memory only once. we should make it into a macro.
+ /* the brigade could be anything, so taint it */ + SvTAINTED_on(data);
that's a good idea!
Here is the patch (against cvs). I've added comments inlined, so you understand what and why:
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 21:33:20 -0000 @@ -97,46 +97,42 @@ }
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_bucket_brigade *bb;
+ apr_pool_t *pool;
+ apr_size_t length;
apr_status_t status;
char *buffer;
- apr_size_t length;
-
- 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 < 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 (items > 2) {
+ /* APR::Brigade->flatten($p, $length); */
+ length = SvIV(*(MARK+2));
}
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.
+ * 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;
}+ buffer = apr_palloc(pool, length+1); /* +1 for '\0' */
+ status = apr_brigade_flatten(bb, buffer, &length);
+ buffer[length] = '\0'; /* it is going to be in sv */
+
if (status != APR_SUCCESS) {
/* XXX croak?
* note that reading from an empty brigade will return
@@ -144,6 +140,25 @@
*/
return &PL_sv_undef;
}
+ else {
+ SV *data = newSV(0);
+ /* as we have already allocated the buffer, we don't want to
+ * use newSVpvn as it'll copy the buffer second time. But we
+ * can't use sv_usepvn(), designed for this purpose, because
+ * it'll try to Renew to add \n, and buffer is allocated from
+ * the apr_pool, not malloc. so we make our own sv, taking
+ * care of the '\0' business */
+ SvUPGRADE(data, SVt_PV);
+ SvPVX(data) = buffer;
+ SvLEN(data) = length;
+ SvCUR(data) = length;
+ /* XXX: shouldn't this be SvPOK_only_UTF8, and in other
+ * places? add a test with some utf8 data */
+ (void)SvPOK_only_UTF8(data);
+
+ /* the brigade could be anything, so taint it */
+ SvTAINTED_on(data);- return newSVpvn(buffer, length); + return data; + } }
__________________________________________________________________ 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]
