Re: Apache.xs patch for get_client_block

[Doug MacEachern]

On 31 Aug 2000 [EMAIL PROTECTED] wrote:

> The mod_perl implementation of get_client_block has a memory leak.
> The following patch should keep it from from pissing in r->pool.

thanks joe.  i don't see how allocating from r->pool is a "leak", but
yeah, it is a waste of resources since Perl is going to make it's own
copy.  read_client_block() has similar waste which i've been meaning to
fix.  this patch trims allocations for both by reading directly into
Perl's buffer:

Index: src/modules/perl/Apache.xs
===================================================================
RCS file: /home/cvs/modperl/src/modules/perl/Apache.xs,v
retrieving revision 1.106
diff -u -r1.106 Apache.xs
--- src/modules/perl/Apache.xs  2000/08/31 05:49:06     1.106
+++ src/modules/perl/Apache.xs  2000/08/31 20:33:03
@@ -940,7 +940,7 @@
 void
 read_client_block(r, buffer, bufsiz)
     Apache     r
-    char    *buffer
+    SV    *buffer
     int      bufsiz
 
     PREINIT:
@@ -948,29 +948,31 @@
     int rc;
 
     PPCODE:
-    buffer = (char*)safemalloc(bufsiz);
     if ((rc = setup_client_block(r, REQUEST_CHUNKED_ERROR)) != OK) {
        aplog_error(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, r->server, 
                    "mod_perl: setup_client_block failed: %d", rc);
        XSRETURN_UNDEF;
     }
 
-    if(should_client_block(r)) {
-       nrd = get_client_block(r, buffer, bufsiz);
-       r->read_length = 0;
+    if (should_client_block(r)) {
+        SvUPGRADE(buffer, SVt_PV);
+        SvGROW(buffer, bufsiz+1);
+        nrd = get_client_block(r, SvPVX(buffer), bufsiz);
+        r->read_length = 0;
     } 
 
     if (nrd > 0) {
-       XPUSHs(sv_2mortal(newSViv((long)nrd)));
-       sv_setpvn((SV*)ST(1), buffer, nrd);
+        XPUSHs(sv_2mortal(newSViv((long)nrd)));
 #ifdef PERL_STASH_POST_DATA
-        table_set(r->subprocess_env, "POST_DATA", buffer);
+        table_set(r->subprocess_env, "POST_DATA", SvPVX(buffer));
 #endif
-        safefree(buffer);
-       SvTAINTED_on((SV*)ST(1));
+        SvCUR_set(buffer, nrd);
+        *SvEND(buffer) = '\0';
+        SvPOK_on(buffer);
+        SvTAINTED_on(buffer);
     } 
     else {
-       ST(1) = &sv_undef;
+        sv_setsv(buffer, &sv_undef);
     }
 
 int
@@ -985,22 +987,25 @@
 void
 get_client_block(r, buffer, bufsiz)
     Apache     r
-    char    *buffer
+    SV    *buffer
     int      bufsiz
 
     PREINIT:
     long nrd = 0;
 
     PPCODE:
-    buffer = (char*)palloc(r->pool, bufsiz);
-    nrd = get_client_block(r, buffer, bufsiz);
+    SvUPGRADE(buffer, SVt_PV);
+    SvGROW(buffer, bufsiz+1);
+    nrd = get_client_block(r, SvPVX(buffer), bufsiz);
     if ( nrd > 0 ) {
        XPUSHs(sv_2mortal(newSViv((long)nrd)));
-       sv_setpvn((SV*)ST(1), buffer, nrd);
-       SvTAINTED_on((SV*)ST(1));
+        SvCUR_set(buffer, nrd);
+        *SvEND(buffer) = '\0';
+        SvPOK_on(buffer);
+        SvTAINTED_on(buffer);
     } 
     else {
-       ST(1) = &sv_undef;
+       sv_setsv(ST(1), &sv_undef);
     }
 
 int