Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:22653
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
To: "'Sara Golemon'" <pollita@php.net>,
	<internals@lists.php.net>
Date: Mon, 3 Apr 2006 16:48:43 +0400
Message-ID: <001401c6571c$f4b262b0$6e02a8c0@thinkpad>
MIME-Version: 1.0
Content-Type: text/plain;
	charset="US-ASCII"
Content-Transfer-Encoding: 7bit
In-Reply-To: <000e01c656b2$98e4b290$88051fac@OHRLVN4523SG>
Importance: Normal
Subject: RE: [PHP-DEV] RETURN_RT_STRING() and family leakage
From: dmitry@zend.com ("Dmitry Stogov")
References: <000e01c656b2$98e4b290$88051fac@OHRLVN4523SG>

Hi Sara,

Your first solution will not work. String passed to ZVAL_RETURN_RT_STRING()
may be not allocated by emalloc().

The second solution will work.

ZVAL_RETURN_RT_STRINGL(str, len, duplicate) -> ZVAL_RETURN_RT_STRINGL(str,
len, duplicate, auto_free)

3) It is possible to reuse "duplicate" argument
0 - don't duplicate
1 - duplicate
2 - duplicate and free

Thanks. Dmitry.

> -----Original Message-----
> From: Sara Golemon [mailto:pollita@php.net] 
> Sent: Monday, April 03, 2006 4:07 AM
> To: internals@lists.php.net
> Subject: [PHP-DEV] RETURN_RT_STRING() and family leakage
> 
> 
> I was going through failing tests today and noticed some which failed 
> because of memory leaks using code like the following:
> 
>    RETURN_RT_STRING(s, 0);
> }
> 
> For non-unicode mode this is functionally identical to:
> 
>   RETURN_STRING(s, 0);
> }
> 
> However for unicode mode, this binary value is translated to 
> unicode using 
> (effectively) this:
> 
>   {
>     UChar *u;
>     int u_len;
>     UErrorCode status = U_ZERO_ERROR;
>     
> zend_convert_to_string(ZEND_U_CONVERTER(UG(runtime_encoding_conv)),
>                            &u, &u_len, s, strlen(s), &status);
>     RETURN_UNICODE(u, 0);
>   }
> }
> 
> The trouble with this, is that the original s value winds up 
> getting leaked. 
> I noticed a few other spots in the source where this is 
> handled as such:
> 
>   RETVAL_RT_STRING(s, 0);
>   if (UG(unicode)) {
>     efree(s);
>   }
>   return;
> }
> 
> Which is, of course, perfectly valid, but it feels a bit 
> cludgy to me as 
> it's a little inconsistent with the normal 
> RETURN_STRING()/RETURN_UNICODE() 
> semantics when it comes to "giving away" the variable.  It's 
> also a problem 
> for the RETURN_RT_STRING(s, 0); usage in general as there's 
> no point after 
> the macro to free the original var.
> 
> I see two solutions:
> 
> (1) Modify the RETURN_RT_STRING(L)() macros to the following:
> 
> #define RETURN_RT_STRING(t, duplicate) \
>  { RETVAL_RT_STRING(t, duplicate); if (duplicate && 
> UG(unicode)) efree(t); 
> return; }
> 
> (2) Modify ZVAL_U_STRINGL() to:
> 
> #define ZVAL_U_STRINGL(conv, z, s, l, duplicate, auto_free) \
>     if (UG(unicode)) { \
>         UErrorCode status = U_ZERO_ERROR; \
>         UChar *u_str; \
>         int u_len; \
>         zend_convert_to_unicode(conv, &u_str, &u_len, s, l, 
> &status); \
>         if (auto_free && !duplicate) { \
>             efree(s); \
>         } \
>         ZVAL_UNICODEL(z, u_str, u_len, 0); \
>     } else { \
>         char *__s=(s); int __l=l;   \
>         Z_STRLEN_P(z) = __l;        \
>         Z_STRVAL_P(z) = (duplicate?estrndup(__s, __l):__s); \
>         Z_TYPE_P(z) = IS_STRING;    \
>     }
> 
> Along with changes to the (ZVAL|RETVAL)_RT_STRING() macros: 
> to support the 
> additional auto_free option (with RETURN_* assuming auto_free=1).
> 
> The first option solves the one real problem with the current 
> implementation 
> by making the uncatchable RETURN_RT_STRING() macros free the 
> original char* 
> as needed, whereas the latter takes that work away from the 
> calling scope at 
> the cost of adding to the proto and requiring more work to go 
> back and clean 
> up existing uses.
> 
> Thoughts?
> 
> -Sara
> 
> -- 
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
> 
> 
>