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.
well, I was just doing it the way it's done elsewhere.
show me where?
but thanks for cleaning it up :)
Yours was clean. It just wasn't efficient.
+ 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 */ +
+ 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);
if this is the way to do it, I guess it should be done this way everywhere else that we are growing the SV prior to population? for instance modperl_io_apache.c calls apr_brigade_flatten after growing the SV (in Apache__RequestIO.h). in fact, grep for mpxs_sv_grow - it looks like each time it could be rewritten allocate the buffer via APR then copy the result to the SV instead of populating SvPVX indirectly.
Not quite so, please take a look at the code again:
mpxs_Apache__RequestRec_read calls:
mpxs_sv_grow(bufsv, len+offset);
at this moment bufsv's PVX field is already of size 'len+offset'. Next:
total = modperl_request_read(aTHX_ r, SvPVX(bufsv)+offset, len);
it passes a pointer to an already allocated buffer, which is next passed to:
tmp = buffer; ... apr_brigade_flatten(bb, tmp, &read);
in modperl_request_read(), which does *not* allocates any new memory. Your case was different as you were using pflatten which was allocating the memory internally, then you were copying it in to the sv.
If you find any other places where we duplicate memory please let me know. It certainly should be fixed.
It easy to check - if SvPVX is passed to some function after sv_grow, then it doesn't allocate anything new.
__________________________________________________________________ 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]
