Mailing-List: contact dev-help@perl.apache.org; run by ezmlm
Precedence: bulk
Message-ID: <3EC81424.6010308@stason.org>
Date: Mon, 19 May 2003 09:15:48 +1000
From: Stas Bekman <stas@stason.org>
Organization: Hope, Humanized
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.1) Gecko/20020826
MIME-Version: 1.0
To: "Philippe M. Chiasson" <gozer@cpan.org>
Cc: dev@perl.apache.org
Subject: Re: [patch] C implementation of $r->content + rfc on the name
References: <3EC34A08.2020400@stason.org>
 <1053085699.4416.32.camel@shou.sg.ectoplasm.org>
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit

Philippe M. Chiasson wrote:

>>+        rc = apr_brigade_pflatten(bb, &buffer, &bufsiz, r->pool);
> 
> 
> No need to set bufsiz to HUGE_STRING_LEN (8192 bytes), since from the
> source of apr_brigade_pflatten, it will allocate apr_brigade_length(bb)
> from the pool, then overwrite bufsize with what it read.

Correct, I've first tried to implement it with _flatter (without the pool) and 
then forgot to remove that.

>>+        if (rc != APR_SUCCESS) {
>>+            apr_brigade_destroy(bb);
>>+            Perl_croak(aTHX_ modperl_apr_strerror(rc));
>>+        }
>>+
>>+        // XXX: more efficient way?
>>+        sv_catpvn(sv, buffer, bufsiz);
> 
> 
> So if we have a file upload POST, for example of 4MB,
> apr_brigade_pflatten() will pcalloc 4MB from the pool, then sv_catpvn
> will in turn malloc another 4MB.. Doesn't sound super cool.

True.

> Why not implement your own pflatten equivalent
> 
> apr_status_t modperl_brigade_sv_flatten(pTHX_ apr_bucket_brigade *bb, SV
> *sv) {
>                                                
> {
>     apr_off_t actual;
>     apr_size_t total;
>     apr_status_t rv;
> 
>     /* XXX: 1: triggers reads on unknown size buckets */
>     apr_brigade_length(bb, 1, &actual);
>     total = (apr_size_t)actual;
>     
>     return apr_brigade_flatten(bb, SvGROW(sv, total), &total);
> }
> 
> Something like that, to at least avoid allocating the storage twice.

right, I've missed the _length API :(

>>+        apr_brigade_cleanup(bb);
>>+    }
>>+    while (!seen_eos);
> 
> 
> Seems to me like a potential infinite loop waiting to happen. What if we
> never see an EOS ?

That was copied from mod_cgi. If we have an infinite loop, than one of the 
custom filters is broken.

>>+    apr_brigade_destroy(bb);
>>+
>>+    return sv;
>>+}
> 
> 
> Hey! Shouldn't we stash that sv in $r somewhere, because it would be
> nice to be able to read $r->content more than only once.

I don't think we should, this is how it was in mp1. If you want to stash it 
away, you do it in your code.

__________________________________________________________________
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


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org