Re: [ZEND-ENGINE-CVS] cvs: ZendEngine2(PHP_5_3) / zend_API.c zend_execute_API.c zend_vm_def.h

Hello Dmitry, Monday, February 4, 2008, 12:10:29 PM, you wrote: > I just see that the patches weren't be tested enough and I see unicode > issues in the HEAD. I hope your changes don't have other issues, but I > cannot be sure (it is very hard to review changes that distributed into > ten different patches). > Anyway, I don't like to waste my time fixing or reverting unfinished > work. I would prefer you to fix tests and issues in reasonable time. just like we all usually do, don't we? > Thanks. Dmitry. > Marcus Boerger wrote: >> Hello Dmitry, >> >> it is a fix in severity and message clarification. There is nothing broken >> here. Nothing to see. And yes unlike you we guys are not working all day on >> PHP so I cannot be 100% correct. Also this patch has been reviewd and >> discussed. As always if you do not like my work, feel free to revert it. >> >> marcus >> >> Monday, February 4, 2008, 10:59:29 AM, you wrote: >> >>> Hi Marcus, >> >>> After all these changes about 40 tests are broken in HEAD and several in >>> PHP_5_3. >> >>> Looking into the patches I'm not sure about all %s format modifiers and >>> Z_STRVAL() macros is HEAD, as them break Unicode mode. >> >>> It is very annoying to find ZE in such a state. :( >> >>> If you couldn't fix it fast, I would prefer to ask you to revert your >>> changes because they too big and too incomplete. Then send the proposed >>> patch to 2internals for review, and then commit the compete and tested >>> patch. >> >>> I would like to review you changes, but now they are distributed over >>> ten incomplete patches which are hard to review. >> >>> Thanks. Dmitry. >> >>> Marcus Boerger wrote: >>>> helly Sat Feb 2 15:48:04 2008 UTC >>>> >>>> Modified files: (Branch: PHP_5_3) >>>> /ZendEngine2 zend_API.c zend_execute_API.c zend_vm_def.h >>>> Log: >>>> - MFH Add comment that explains why we need E_ERROR in those cases >>>> >>>> >>>> http://cvs.php.net/viewvc.cgi/ZendEngine2/zend_API.c?r1=1.296.2.27.2.34.2.23&r2=1.296.2.27.2.34.2.24&diff_format=u >>>> Index: ZendEngine2/zend_API.c >>>> diff -u ZendEngine2/zend_API.c:1.296.2.27.2.34.2.23 >>>> ZendEngine2/zend_API.c:1.296.2.27.2.34.2.24 >>>> --- ZendEngine2/zend_API.c:1.296.2.27.2.34.2.23 Sat Feb 2 15:18:30 2008 >>>> +++ ZendEngine2/zend_API.c Sat Feb 2 15:48:04 2008 >>>> @@ -18,7 +18,7 @@ >>>> +----------------------------------------------------------------------+ >>>> */ >>>> >>>> -/* $Id: zend_API.c,v 1.296.2.27.2.34.2.23 2008/02/02 15:18:30 helly Exp $ */ >>>> +/* $Id: zend_API.c,v 1.296.2.27.2.34.2.24 2008/02/02 15:48:04 helly Exp $ */ >>>> >>>> #include "zend.h" >>>> #include "zend_execute.h" >>>> @@ -2406,6 +2406,7 @@ >>>> severity = E_STRICT; >>>> verb = "should not"; >>>> } else { >>>> + /* An internal function assumes $this is >>>> present and won't check >>>> that. So PHP would crash by allowing the call. */ >>>> severity = E_ERROR; >>>> verb = "cannot"; >>>> } >>>> >>>> http://cvs.php.net/viewvc.cgi/ZendEngine2/zend_execute_API.c?r1=1.331.2.20.2.24.2.24&r2=1.331.2.20.2.24.2.25&diff_format=u >>>> Index: ZendEngine2/zend_execute_API.c >>>> diff -u ZendEngine2/zend_execute_API.c:1.331.2.20.2.24.2.24 >>>> ZendEngine2/zend_execute_API.c:1.331.2.20.2.24.2.25 >>>> --- ZendEngine2/zend_execute_API.c:1.331.2.20.2.24.2.24 Sat Feb 2 13:56:59 >>>> 2008 >>>> +++ ZendEngine2/zend_execute_API.c Sat Feb 2 15:48:04 2008 >>>> @@ -17,7 +17,7 @@ >>>> +----------------------------------------------------------------------+ >>>> */ >>>> >>>> -/* $Id: zend_execute_API.c,v 1.331.2.20.2.24.2.24 2008/02/02 13:56:59 helly Exp $ >>>> */ >>>> +/* $Id: zend_execute_API.c,v 1.331.2.20.2.24.2.25 2008/02/02 15:48:04 helly Exp $ >>>> */ >>>> >>>> #include <stdio.h> >>>> #include <signal.h> >>>> @@ -1051,6 +1051,7 @@ >>>> severity = E_STRICT; >>>> verb = "should not"; >>>> } else { >>>> + /* An internal function assumes $this is present and >>>> won't check that. So >>>> PHP would crash by allowing the call. */ >>>> severity = E_ERROR; >>>> verb = "cannot"; >>>> } >>>> >>>> http://cvs.php.net/viewvc.cgi/ZendEngine2/zend_vm_def.h?r1=1.59.2.29.2.48.2.34&r2=1.59.2.29.2.48.2.35&diff_format=u >>>> Index: ZendEngine2/zend_vm_def.h >>>> diff -u ZendEngine2/zend_vm_def.h:1.59.2.29.2.48.2.34 >>>> ZendEngine2/zend_vm_def.h:1.59.2.29.2.48.2.35 >>>> --- ZendEngine2/zend_vm_def.h:1.59.2.29.2.48.2.34 Sat Feb 2 13:56:59 2008 >>>> +++ ZendEngine2/zend_vm_def.h Sat Feb 2 15:48:04 2008 >>>> @@ -18,7 +18,7 @@ >>>> +----------------------------------------------------------------------+ >>>> */ >>>> >>>> -/* $Id: zend_vm_def.h,v 1.59.2.29.2.48.2.34 2008/02/02 13:56:59 helly Exp $ */ >>>> +/* $Id: zend_vm_def.h,v 1.59.2.29.2.48.2.35 2008/02/02 15:48:04 helly Exp $ */ >>>> >>>> /* If you change this file, please regenerate the zend_vm_execute.h and >>>> * zend_vm_opcodes.h files by running: >>>> @@ -1975,6 +1975,7 @@ >>>> severity = E_STRICT; >>>> verb = "should not"; >>>> } else { >>>> + /* An internal function assumes $this is present and >>>> won't check that. So >>>> PHP would crash by allowing the call. */ >>>> severity = E_ERROR; >>>> verb = "cannot"; >>>> } >>>> @@ -2082,6 +2083,7 @@ >>>> zend_error(E_STRICT, "Non-static method %s::%s() should >>>> not be called statically", >>>> EX(function_state).function->common.scope->name, >>>> EX(function_state).function->common.function_name); >>>> } else { >>>> /* FIXME: output identifiers properly */ >>>> + /* An internal function assumes $this is present and >>>> won't check that. So PHP would >>>> crash by allowing the call. */ >>>> zend_error_noreturn(E_ERROR, "Non-static method %s::%s() >>>> cannot be called >>>> statically", EX(function_state).function->common.scope->name, >>>> EX(function_state).function->common.function_name); >>>> } >>>> } >>>> >> >> >> >> >> Best regards, >> Marcus >> Best regards, Marcus