Re: [PATCH] A suggested fix for bug #19943 (Re: [PHP-DEV] bug #19943)

> The patch does work and memory leak goes away after it is applied. There is > another problem that I've just noticed. If you increase the loop from 10 to > 100 then you'll see that the $ragged[$count]['idx'] value is always the 1st > character of the $ragged[$count] value. > If a var_dump of an array is made you'll see that the array stucture looks > something like this: --snip-- > And so on, no mention of 'idx'. I believe since idx is not found,value of 0 is > used, which causes the look up to go after $ragged[$count][0], returning the > 1st character of the $ragged[$count] value. > IMHO the fix for this problem should give the user a warning message > indicating that they are not doing something correctly. ZE already would give > you such an error, if were you try: IMO this is a expected behaviour at least. Because although this is a deprecated manner, [] can be used in place of {} to specify an offset to a character of a string in favour of BC, so when $ragged[0] has a string value, the reference should be treated as that to a character rather than to an array element. And engine may well complain about the validity when $ragged[0] doesn't hold a string value, nor an array value. > P.S. After your patch is applied the test script you've posted, results in the > following error: > --------------------------------------- > Zend/zend_execute.c(376) : Block 0x084EC268 status: > Beginning: Cached (allocated on Zend/zend_execute.c:1985, 44 bytes) > End: OK > --------------------------------------- Perhaps my patch was not sufficient... A revised patch is attached. Please try this one, and if you like the patch, then commit it with extreme care :) Moriyoshi > > On November 5, 2002 05:48 pm, Moriyoshi Koizumi wrote: > > The attached patch may be a fix for that bug. > > Hope it works. > > > > Moriyoshi > > > > Moriyoshi Koizumi <moriyoshi@at.wakwak.com> wrote: > > > I looked into it, and I've found you really got a point. > > > > > > --------------------------- > > > <?php > > > $ragged = array(); > > > $ragged[0] = "a"; > > > $ragged[0][0] = array("1"); > > > var_dump($ragged); > > > ?> > > > --------------------------- > > > > > > The first script causes no leaks, while the second does: > > > --------------------------- > > > <?php > > > $ragged = array(); > > > $ragged[0] = "a"; > > > $ragged[0][0] = (string)array("1"); > > > var_dump($ragged); > > > ?> > > > --------------------------- > > > > > > Thanks for your insight. > > > > > > > > > Moriyoshi > > > > > > "Ilia A." <ilia@prohost.org> wrote: > > > > I agree with you that this bug may not be critical, I am not certain > > > > why it was marked as such, however I do believe this is a bug that > > > > should be fixed. If my understand of the situation is correct, the > > > > memory leak is the result of original $ragged[$count] = "$count"; > > > > expression. At this point ZE creates a ZVAL that contains a 2 byte > > > > string value, number + \0. > > > > When $ragged[$count]['idx'] = 'ragged '.$count; is executed, > > > > the > > > > original zval containing a string is converted to an array. During this > > > > conversion the string value is not freed and the result is a memory > > > > leak. > > > > > > > > Ilia > > > > > > > > On November 5, 2002 01:45 pm, Moriyoshi Koizumi wrote: > > > > > This appears not a bug, but an expected behaviour for me. > > > > > Let's change it from "Critial" to "Won't Fix" > > > > > or so on. > > > > > > > > > > --------------------------------------------------------------- > > > > > <?php > > > > > $ragged = array(); > > > > > for ($count = 0; $count < 10; $count++) { > > > > > $ragged[$count] = "$count"; > > > > > $ragged[$count]['idx'] = "$count"; > > > > > } > > > > > for ($count = 0; $count < 10; $count++) { > > > > > printf("single %d: %s\n", $count, $ragged[$count]); > > > > > printf("ragged %d: %s\n", $count, > > > > > $ragged[$count]['idx']); > > > > > } > > > > > ?> > > > > > --------------------------------------------------------------- > > > > > > > > > > The above snippet is actually a variant of the following code: > > > > > --------------------------------------------------------------- > > > > > <?php > > > > > $ragged = array(); > > > > > for ($count = 0; $count < 10; $count++) { > > > > > $ragged[$count] = (string)$count; > > > > > $ragged[$count]{(int)'idx'} = (string)$count; > > > > > } > > > > > for ($count = 0; $count < 10; $count++) { > > > > > printf("single %d: %s\n", $count, $ragged[$count]); > > > > > printf("ragged %d: %s\n", $count, > > > > > $ragged[$count]{(int)'idx'}); > > > > > } > > > > > ?> > > > > > --------------------------------------------------------------- > > > > > Then, "Cannot use a scalar value as an array" warnings are due to > > > > > applications of braces for non-string variables. > > > > > But I can't still explain why leaks occured. > > > > > > > > > > Moriyoshi > > > > > > > > -- > > > > PHP Development Mailing List <fPà,Ò·x > > > > JËz#ëçj¤http://www.php.net/> > > > > To unsubscribe, visit: > > > > http://www.php.net/unsub.php > > > > > > -- > > > PHP Development Mailing List > > > <http://www.php.net/> > > > To unsubscribe, visit: http://www.php.net/unsub.php >

Index: zend_execute.c =================================================================== RCS file: /repository/Zend/zend_execute.c,v retrieving revision 1.315 diff -u -r1.315 zend_execute.c --- zend_execute.c 3 Nov 2002 15:16:45 -0000 1.315 +++ zend_execute.c 6 Nov 2002 02:11:48 -0000 @@ -371,9 +371,9 @@ T->EA.data.str_offset.str->value.str.val[T->EA.data.str_offset.offset] = final_value->value.str.val[0]; if (op2 - && op2->op_type == IS_VAR - && value==&Ts[op2->u.var].tmp_var) { - STR_FREE(value->value.str.val); + && op2->op_type != IS_VAR + && final_value == &Ts[op2->u.var].tmp_var) { + STR_FREE(final_value->value.str.val); } if (final_value == &tmp) { zval_dtor(final_value);