[PHP-DEV] Re: [ZEND-ENGINE-CVS] cvs: ZendEngine2(PHP_5_3) / zend_execute.c zend_vm_def.h zend_vm_execute.h

Jani Taskinen Sun, 27 Jul 2008 02:46:34 -0700

Marcus Boerger kirjoitti:

Hello Internals,


  apparently overloaded objects do not need to implement property access
and we issue an E_NOTICE in case someone tries to none-the-less. Dmirty
thankfully made this consistent for all handlers now. However this raised a
question on my side, whether we should increase the severity to E_WARNING
or even better E_RECOVERABLE_ERROR. To me the latter choice makes the most
sense as trying to is most likely a severe issue in the software. For
example someone create a database abstraction and then for a new db that
has a C level implementation that allows to use the classes directly,
probably a third party implementation, the properties are not implemneted.
Since the tests were done using the other one errors due to property
handling are probably noticed too late. And in the described situation
anyway are clear errors rather than notices. And I cannot figure an example
where it wouldn't be the case.


E_RECOVERABLE_ERROR sounds like the best choice.

--Jani

marcus

Saturday, July 26, 2008, 8:43:57 PM, you wrote:

Hi Marcus,

I set the same severity which was used for emptyread_property/write_property.

In case of changing they should be changed too.

I have no preference, so if you are interested in the change pleasediscuss it on @internals.

Thanks. Dmitry.

Marcus Boerger wrote:

Hello Dmitry,

  can we increase the severity here? The user might think he is using a
normal object and relying on the property assignment to work. Imo it should
be an E_WARNING or even better an E_RECOVERABLE_ERROR.

marcus

Saturday, July 26, 2008, 4:08:16 PM, you wrote:

http://cvs.php.net/viewvc.cgi/ZendEngine2/zend_execute.c?r1=1.716.2.12.2.24.2.32&r2=1.716.2.12.2.24.2.33&diff_format=u
Index: ZendEngine2/zend_execute.c
diff -u ZendEngine2/zend_execute.c:1.716.2.12.2.24.2.32
ZendEngine2/zend_execute.c:1.716.2.12.2.24.2.33
--- ZendEngine2/zend_execute.c:1.716.2.12.2.24.2.32     Mon Jul 14 09:49:00 2008
+++ ZendEngine2/zend_execute.c  Sat Jul 26 14:08:10 2008
@@ -17,7 +17,7 @@

+----------------------------------------------------------------------+

*/

-/* $Id: zend_execute.c,v 1.716.2.12.2.24.2.32 2008/07/14 09:49:00 dmitry Exp $ */

+/* $Id: zend_execute.c,v 1.716.2.12.2.24.2.33 2008/07/26 14:08:10 dmitry Exp $ 
*/

#define ZEND_INTENSIVE_DEBUGGING 0@@ -594,6 +594,11 @@

                                *retval = EG(uninitialized_zval_ptr);
                                PZVAL_LOCK(*retval);
                        }
+                       if (value_op->op_type == IS_TMP_VAR) {
+                               FREE_ZVAL(value);
+                       } else if (value_op->op_type == IS_CONST) {
+                               zval_ptr_dtor(&value);
+                       }
                        FREE_OP(free_value);
                        return;
                }
http://cvs.php.net/viewvc.cgi/ZendEngine2/zend_vm_def.h?r1=1.59.2.29.2.48.2.63&r2=1.59.2.29.2.48.2.64&diff_format=u
Index: ZendEngine2/zend_vm_def.h
diff -u ZendEngine2/zend_vm_def.h:1.59.2.29.2.48.2.63
ZendEngine2/zend_vm_def.h:1.59.2.29.2.48.2.64
--- ZendEngine2/zend_vm_def.h:1.59.2.29.2.48.2.63       Sat Jul 26 13:14:00 2008
+++ ZendEngine2/zend_vm_def.h   Sat Jul 26 14:08:11 2008
@@ -18,7 +18,7 @@

+----------------------------------------------------------------------+

*/

-/* $Id: zend_vm_def.h,v 1.59.2.29.2.48.2.63 2008/07/26 13:14:00 dmitry Exp $ */

+/* $Id: zend_vm_def.h,v 1.59.2.29.2.48.2.64 2008/07/26 14:08:11 dmitry Exp $ */

/* If you change this file, please regenerate the zend_vm_execute.h and

  * zend_vm_opcodes.h files by running:
@@ -3479,7 +3479,11 @@
                        if (IS_OP2_TMP_FREE()) {
                                MAKE_REAL_ZVAL_PTR(offset);
                        }

-Z_OBJ_HT_P(*container)->unset_property(*container, offset TSRMLS_CC);

+                       if (Z_OBJ_HT_P(*container)->unset_property) {