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

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. 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