Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:39820
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain zend.com designates 212.25.124.163 as permitted sender)
Message-ID: <489FFDA0.4080206@zend.com>
Date: Mon, 11 Aug 2008 12:51:44 +0400
User-Agent: Thunderbird 2.0.0.16 (Windows/20080708)
MIME-Version: 1.0
To: Marcus Boerger <helly@php.net>
CC: =?ISO-8859-15?Q?Johannes_Schl=FCter?= <johannes@php.net>, 
 Christian Seiler <chris_se@gmx.net>,
 Felipe Pena <felipensp@gmail.com>, 
 php-dev List <internals@lists.php.net>
References: <48886651.80407@gmx.net> <4889F43D.8030609@gmx.net> <4896B0B4.2090801@zend.com> <1217847553.4175.23.camel@goldfinger.johannes.nop> <875726149.20080808234837@marcus-boerger.de> <803831941.20080809003459@marcus-boerger.de> <1199358810.20080811005138@marcus-boerger.de>
In-Reply-To: <1199358810.20080811005138@marcus-boerger.de>
Content-Type: text/plain; charset=ISO-8859-15; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] [PATCH] Closures and reflection improvements
From: dmitry@zend.com (Dmitry Stogov)

Hi Marcus,

I committed the ZE part of your patch with exception for

zend_error(E_ERROR, "Cannot destroy active lambda function");

Catching this error may cause memory corruption and crash.

Thanks. Dmitry.

Marcus Boerger wrote:
> Hello Johannes, Christian, Felipe,
> 
>   and here 5.3 and 6.0 versions that also have most memleaks fixed.
> 
> marcus
> 
> Saturday, August 9, 2008, 12:34:59 AM, you wrote:
> 
>> Hello Christian, Johannes,
> 
>> Friday, August 8, 2008, 11:48:37 PM, you wrote:
> 
>>> Hello Christian,
> 
>>>   I updated your patch for 5.3 due to recent changes. It works pretty fine
>>> for me. Care to test again, especially with your new tests? Note that test
>>> ext/reflection/tests/closures_001.phpt does not work for me and I have no
>>> clue why. Well, besides that obviously the closure class no longer has an
>>> entry for __closure, so it is more a question of how to know when to add
>>> this. Also, it appears you do not have a cvs account, or did I overlook
>>> something?
> 
>>> Modified patch attached, including the updated reflection tests.
> 
>> Ok, it is actually easy to fix, we just check whether the thing is a
>> closure and add the method manually, done. New patch attached.
> 
>> Johannes, this also adresses the switch from E_ERROR to E_RECOVERABLE_ERROR
>> for the closure handlers.
> 
>> best regards
>> marcus
> 
>>> Monday, August 4, 2008, 12:59:13 PM, you wrote:
> 
>>>> Christian,
> 
>>>> On Mon, 2008-08-04 at 11:33 +0400, Dmitry Stogov wrote:
>>>>>>> http://www.christian-seiler.de/temp/php/2008-07-24-reflection/reflection-closure-fixes-5.3.patch 
>>>>>>>
>>>>>>> http://www.christian-seiler.de/temp/php/2008-07-24-reflection/reflection-closure-fixes-6.patch 
> 
>>>> without applying and testing the patch:
> 
>>>> -       zend_hash_apply_with_arguments(&ce->function_table TSRMLS_CC,
>>>> (apply_func_args_t) _addmethod, 3, &ce, return_value, filter);
>>>> +       zend_hash_apply_with_arguments(&ce->function_table TSRMLS_CC,
>>>> (apply_func_args_t) _addmethod, 3, &ce, return_value, filter, intern->obj);
> 
>>>> that looks a bit strange, probably you want 4 instead of 3 now?
> 
>>>> johannes
> 
> 
> 
> 
> 
>>> Best regards,
>>>  Marcus
> 
> 
> 
>> Best regards,
>>  Marcus
> 
> 
> 
> Best regards,
>  Marcus
>