RETURN_RT_STRING() and family leakage View as Markdown

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