Re: RETURN_RT_STRING() and family leakage

Here's what I plan to commit unless I hear otherwise. I went with ZSTR_ as the prefix for the constants as S_ isn't terribly unique. If anyone has a better suggestion, now is the time to voice it. -Sara ----- Original Message ----- From: Dmitry Stogov To: 'Sara Golemon' Cc: internals@lists.php.net Sent: Monday, April 03, 2006 10:02 PM Subject: RE: [PHP-DEV] RETURN_RT_STRING() and family leakage I prefer (2) or (3). (3) requires less changes. #define S_DUPLICATE (1<<0) #define S_AUTO_FREE (1<<1) #define ZVAL_U_STRINGL(conv, z, s, l, flags) \ 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); \ ZVAL_UNICODEL(z, u_str, u_len, 0); \ if (flags & S_AUTO_FREE) {efree(s);} \ } else { \ char *__s=(s); int __l=l; \ Z_STRLEN_P(z) = __l; \ Z_STRVAL_P(z) = ((flags & S_DUPLICATE)?estrndup(__s, __l):__s); \ Z_TYPE_P(z) = IS_STRING; \ } And then we need to change only calls those rally leak. RETURN_RT_STRING(str, 1) -> RETURN_RT_STRING(str, S_DUPLICATE | S_AUTO_FREE) Thanks. Dmitry. > -----Original Message----- > From: Sara Golemon [mailto:pollita@php.net] > Sent: Monday, April 03, 2006 10:15 PM > To: "Dmitry Stogov" > Cc: internals@lists.php.net > Subject: Re: [PHP-DEV] RETURN_RT_STRING() and family leakage > > > >> duplicate should only ever be set to 0 on this (or any of the > >> macros) when > >> the string *is* allocated with emalloc. Otherwise the > enegine would > >> get in trouble freeing it later on. > > > > No. :( > > You can use ZVAL_RT_STRING(&fname, "strlen", 0), then call > > zend_call_function(&fname) and do not destroy fname. > > > Doi, of course, good point... > > Okay, then the options are: > > (1) Assume auto_free for RETURN_RT_STRING(s,0) specifically > as in this case, > my statement above is valid since the engine IS going to try > it eventually. > Leave all other macros alone and make the calling scope > handle the original > string with the existing if (UG(unicode)) efree(s); Not my favorite. > > (2) Expand (ZVAL|RETVAL)_RT_STRING(s, 0) to include auto_free > argument. > This coule be done in conjunction with (1) or in place of it. > > (3) Overload duplicate argument to include (should I > auto-free?) logic. > > (4) Duplicate the macros to one auto-free, and one > non-auto-free version. > > I like the #1/#2 combo personally. > > -Sara > > >

Index: Zend/zend_API.h =================================================================== RCS file: /repository/ZendEngine2/zend_API.h,v retrieving revision 1.239 diff -u -r1.239 zend_API.h --- Zend/zend_API.h 10 Mar 2006 16:35:21 -0000 1.239 +++ Zend/zend_API.h 5 Apr 2006 22:10:55 -0000 @@ -358,27 +358,36 @@ add_assoc_stringl_ex(arg, key, key_len, (char*)(str), length, duplicate); \ } -#define add_assoc_rt_string_ex(arg, key, key_len, str, duplicate) \ +#define ZSTR_DUPLICATE (1<<0) +#define ZSTR_AUTOFREE (1<<1) + +#define add_assoc_rt_string_ex(arg, key, key_len, str, flags) \ if (UG(unicode)) { \ UErrorCode status = U_ZERO_ERROR; \ UChar *u_str; \ int u_len; \ int length = strlen(str); \ zend_convert_to_unicode(ZEND_U_CONVERTER(UG(runtime_encoding_conv)), &u_str, &u_len, str, length, &status); \ + if ((flags) & ZSTR_AUTOFREE) { \ + efree(str); \ + } \ add_assoc_unicodel_ex(arg, key, key_len, u_str, u_len, 0); \ } else { \ - add_assoc_string_ex(arg, key, key_len, (char*)(str), duplicate); \ + add_assoc_string_ex(arg, key, key_len, (char*)(str), (flags) & ZSTR_DUPLICATE); \ } -#define add_assoc_rt_stringl_ex(arg, key, key_len, str, length, duplicate) \ +#define add_assoc_rt_stringl_ex(arg, key, key_len, str, length, flags) \ if (UG(unicode)) { \ UErrorCode status = U_ZERO_ERROR; \ UChar *u_str; \ int u_len; \ zend_convert_to_unicode(ZEND_U_CONVERTER(UG(runtime_encoding_conv)), &u_str, &u_len, str, length, &status); \ + if ((flags) & ZSTR_AUTOFREE) { \ + efree(str); \ + } \ add_assoc_unicodel_ex(arg, key, key_len, u_str, u_len, 0); \ } else { \ - add_assoc_stringl_ex(arg, key, key_len, (char*)(str), length, duplicate); \ + add_assoc_stringl_ex(arg, key, key_len, (char*)(str), length, (flags) & ZSTR_DUPLICATE); \ } #define add_assoc_long(__arg, __key, __n) add_assoc_long_ex(__arg, __key, strlen(__key)+1, __n) @@ -519,27 +528,33 @@ add_next_index_stringl(arg, (char*)(str), length, duplicate); \ } -#define add_next_index_rt_string(arg, str, duplicate) \ +#define add_next_index_rt_string(arg, str, flags) \ if (UG(unicode)) { \ UErrorCode status = U_ZERO_ERROR; \ UChar *u_str; \ int u_len; \ int length = strlen(str); \ zend_convert_to_unicode(ZEND_U_CONVERTER(UG(runtime_encoding_conv)), &u_str, &u_len, str, length, &status); \ + if ((flags) & ZSTR_AUTOFREE) { \ + efree(str); \ + } \ add_next_index_unicodel(arg, u_str, u_len, 0); \ } else { \ - add_next_index_string(arg, (char*)(str), duplicate); \ + add_next_index_string(arg, (char*)(str), (flags) & ZSTR_DUPLICATE); \ } -#define add_next_index_rt_stringl(arg, str, length, duplicate) \ +#define add_next_index_rt_stringl(arg, str, length, flags) \ if (UG(unicode)) { \ UErrorCode status = U_ZERO_ERROR; \ UChar *u_str; \ int u_len; \ zend_convert_to_unicode(ZEND_U_CONVERTER(UG(runtime_encoding_conv)), &u_str, &u_len, str, length, &status); \ + if ((flags) & ZSTR_AUTOFREE) { \ + efree(str); \ + } \ add_next_index_unicodel(arg, u_str, u_len, 0); \ } else { \ - add_next_index_stringl(arg, (char*)(str), length, duplicate); \ + add_next_index_stringl(arg, (char*)(str), length, (flags) & ZSTR_DUPLICATE); \ } ZEND_API int add_get_assoc_string_ex(zval *arg, char *key, uint key_len, char *str, void **dest, int duplicate); @@ -707,40 +722,46 @@ Z_TYPE_P(z) = IS_STRING; \ } -#define ZVAL_U_STRING(conv, z, s, duplicate) \ +#define ZVAL_U_STRING(conv, z, s, flags) \ if (UG(unicode)) { \ UErrorCode status = U_ZERO_ERROR; \ UChar *u_str; \ int u_len; \ uint length = strlen(s); \ zend_convert_to_unicode(conv, &u_str, &u_len, s, length, &status); \ + if ((flags) & ZSTR_AUTOFREE) { \ + efree(s); \ + } \ ZVAL_UNICODEL(z, u_str, u_len, 0); \ } else { \ char *__s=(s); \ Z_STRLEN_P(z) = strlen(__s); \ - Z_STRVAL_P(z) = (duplicate?estrndup(__s, Z_STRLEN_P(z)):__s); \ + Z_STRVAL_P(z) = (((flags) & ZSTR_DUPLICATE) ? estrndup(__s, Z_STRLEN_P(z)) : __s); \ Z_TYPE_P(z) = IS_STRING; \ } -#define ZVAL_U_STRINGL(conv, z, s, l, duplicate) \ +#define ZVAL_U_STRINGL(conv, z, s, l, flags) \ 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 ((flags) & ZSTR_AUTOFREE) { \ + 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_STRVAL_P(z) = (((flags) & ZSTR_DUPLICATE) ? estrndup(__s, __l) : __s); \ Z_TYPE_P(z) = IS_STRING; \ } -#define ZVAL_RT_STRING(z, s, duplicate) \ - ZVAL_U_STRING(ZEND_U_CONVERTER(UG(runtime_encoding_conv)), z, s, duplicate) +#define ZVAL_RT_STRING(z, s, flags) \ + ZVAL_U_STRING(ZEND_U_CONVERTER(UG(runtime_encoding_conv)), z, s, flags) -#define ZVAL_RT_STRINGL(z, s, l, duplicate) \ - ZVAL_U_STRINGL(ZEND_U_CONVERTER(UG(runtime_encoding_conv)), z, s, l, duplicate) +#define ZVAL_RT_STRINGL(z, s, l, flags) \ + ZVAL_U_STRINGL(ZEND_U_CONVERTER(UG(runtime_encoding_conv)), z, s, l, flags) #define ZVAL_UNICODE(z, u, duplicate) { \ UChar *__u=(u); \ @@ -825,8 +846,8 @@ #define RETVAL_ASCII_STRINGL(s, l, duplicate) ZVAL_ASCII_STRINGL(return_value, s, l, duplicate) #define RETVAL_U_STRING(conv, s, duplicate) ZVAL_U_STRING(conv, return_value, s, duplicate) #define RETVAL_U_STRINGL(conv, s, l, duplicate) ZVAL_U_STRINGL(conv, return_value, s, l, duplicate) -#define RETVAL_RT_STRING(s, duplicate) ZVAL_RT_STRING(return_value, s, duplicate) -#define RETVAL_RT_STRINGL(s, l, duplicate) ZVAL_RT_STRINGL(return_value, s, l, duplicate) +#define RETVAL_RT_STRING(s, flags) ZVAL_RT_STRING(return_value, s, flags) +#define RETVAL_RT_STRINGL(s, l, flags) ZVAL_RT_STRINGL(return_value, s, l, flags) #define RETVAL_EMPTY_STRING() ZVAL_EMPTY_STRING(return_value) #define RETVAL_UNICODE(u, duplicate) ZVAL_UNICODE(return_value, u, duplicate) #define RETVAL_UNICODEL(u, l, duplicate) ZVAL_UNICODEL(return_value, u, l, duplicate) @@ -859,8 +880,8 @@ #define RETURN_ASCII_STRINGL(t, l, duplicate) { RETVAL_ASCII_STRINGL(t, l, duplicate); return; } #define RETURN_U_STRING(conv, t, duplicate) { RETVAL_U_STRING(conv, t, duplicate); return; } #define RETURN_U_STRINGL(conv, t, l, duplicate) { RETVAL_U_STRINGL(conv, t, l, duplicate); return; } -#define RETURN_RT_STRING(t, duplicate) { RETVAL_RT_STRING(t, duplicate); return; } -#define RETURN_RT_STRINGL(t, l, duplicate) { RETVAL_RT_STRINGL(t, l, duplicate); return; } +#define RETURN_RT_STRING(t, flags) { RETVAL_RT_STRING(t, flags); return; } +#define RETURN_RT_STRINGL(t, l, flags) { RETVAL_RT_STRINGL(t, l, flags); return; } #define SET_VAR_STRING(n, v) { \ { \