Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:33041
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
To: internals@lists.php.net
Date: Tue, 6 Nov 2007 12:51:19 -0800
Message-ID: <20071106125119.6d073e9f@h4x0r.tss>
Organization: The Selling Source, Inc.
Mime-Version: 1.0
Content-Type: multipart/mixed; boundary="MP_r/Ft77yW8YtYy/AOKc87Qo8"
Subject: [Patch] Allow __sleep to return NULL
From: andrew.minerd@sellingsource.com (Andrew Minerd)

--MP_r/Ft77yW8YtYy/AOKc87Qo8
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

I've attached a patch (both for HEAD and the PHP_5_3 branch) that allows the magic __sleep() method to return NULL to continue the normal serialization process (all members are serialized). This allows the __sleep() method to do meaningful cleanup without having to resort to Reflection overhead. The patch retains the original behavior (E_NOTICE) if anything other than an array or NULL is returned from the function.

I check the results of a "make test" on the PHP_5_3 branch with and without the patch. I skipped the results from HEAD because there were so many unrelated failures it was pointless. The one additional failure in 5.3 with the patch is actually a reverse unit test. I've included a patch (separately, because it's the same for HEAD and PHP_5_3) to correct the expected result from this test.

Thanks,

Andrew
--MP_r/Ft77yW8YtYy/AOKc87Qo8
Content-Type: text/x-patch; name=andrewm.sleep_null.53.patch
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment; filename=andrewm.sleep_null.53.patch

Index: ext/standard/var.c
===================================================================
RCS file: /repository/php-src/ext/standard/var.c,v
retrieving revision 1.203.2.7.2.18.2.4
diff -u -r1.203.2.7.2.18.2.4 var.c
--- ext/standard/var.c	2 Nov 2007 19:40:39 -0000	1.203.2.7.2.18.2.4
+++ ext/standard/var.c	6 Nov 2007 19:44:51 -0000
@@ -726,19 +726,21 @@
 					ZVAL_STRINGL(&fname, "__sleep", sizeof("__sleep") - 1, 0);
 					res = call_user_function_ex(CG(function_table), &struc, &fname, &retval_ptr, 0, 0, 1, NULL TSRMLS_CC);
 
-					if (res == SUCCESS && !EG(exception)) {
-						if (retval_ptr) {
-							if (HASH_OF(retval_ptr)) {
-								php_var_serialize_class(buf, struc, retval_ptr, var_hash TSRMLS_CC);
-							} else {
-								php_error_docref(NULL TSRMLS_CC, E_NOTICE, "__sleep should return an array only containing the names of instance-variables to serialize");
-								/* we should still add element even if it's not OK,
-								 * since we already wrote the length of the array before */
-								smart_str_appendl(buf,"N;", 2);
-							}
+					if (res == SUCCESS && !EG(exception) && retval_ptr) {
+						/* only serialize using the return value
+							if it's an array, allow NULLs to fall through */
+						if (HASH_OF(retval_ptr)) {
+							php_var_serialize_class(buf, struc, retval_ptr, var_hash TSRMLS_CC);
 							zval_ptr_dtor(&retval_ptr);
+							return;
+						} else if (Z_TYPE_P(retval_ptr) != IS_NULL) {
+							php_error_docref(NULL TSRMLS_CC, E_NOTICE, "__sleep should return an array only containing the names of instance-variables to serialize");
+							/* we should still add element even if it's not OK,
+								since we already wrote the length of the array before */
+							smart_str_appendl(buf,"N;", 2);
+							zval_ptr_dtor(&retval_ptr);
+							return;
 						}
-						return;
 					}
 				}
 

--MP_r/Ft77yW8YtYy/AOKc87Qo8
Content-Type: text/x-patch; name=andrewm.sleep_null.HEAD.patch
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment; filename=andrewm.sleep_null.HEAD.patch

Index: ext/standard/var.c
===================================================================
RCS file: /repository/php-src/ext/standard/var.c,v
retrieving revision 1.271
diff -u -r1.271 var.c
--- ext/standard/var.c	2 Nov 2007 09:43:04 -0000	1.271
+++ ext/standard/var.c	6 Nov 2007 19:22:24 -0000
@@ -1003,19 +1003,24 @@
 					ZVAL_ASCII_STRINGL(&fname, "__sleep", sizeof("__sleep") - 1, 1);
 					res = call_user_function_ex(CG(function_table), &struc, &fname, &retval_ptr, 0, 0, 1, NULL TSRMLS_CC);
 					zval_dtor(&fname);
-					if (res == SUCCESS && !EG(exception)) {
-						if (retval_ptr) {
-							if (HASH_OF(retval_ptr)) {
-								php_var_serialize_class(buf, struc, retval_ptr, var_hash TSRMLS_CC);
-							} else {
-								php_error_docref(NULL TSRMLS_CC, E_NOTICE, "__sleep should return an array only containing the names of instance-variables to serialize");
-								/* we should still add element even if it's not OK,
-								 * since we already wrote the length of the array before */
-								smart_str_appendl(buf,"N;", 2);
-							}
+
+					if (res == SUCCESS && !EG(exception) && retval_ptr) {
+						/* only serialize using the return value
+							if it's an array, allow NULLs to fall through */
+						if (HASH_OF(retval_ptr)) {
+							php_var_serialize_class(buf, struc, retval_ptr, var_hash TSRMLS_CC);
+							zval_ptr_dtor(&retval_ptr);
+							return;
+						} else if (Z_TYPE_P(retval_ptr) != IS_NULL) {
+							php_error_docref(NULL TSRMLS_CC, E_NOTICE, "__sleep should return an array "
+									 "containing the names of instance-variables to "
+									 "serialize, or NULL.");
+							/* we should still add element even if it's not OK,
+								since we already wrote the length of the array before */
+							smart_str_appendl(buf,"N;", 2);
 							zval_ptr_dtor(&retval_ptr);
+							return;
 						}
-						return;
 					}
 				}
 

--MP_r/Ft77yW8YtYy/AOKc87Qo8
Content-Type: text/x-patch; name=andrewm.sleep_null.test.patch
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment; filename=andrewm.sleep_null.test.patch

Index: ext/standard/tests/serialize/bug21957.phpt
===================================================================
RCS file: /repository/php-src/ext/standard/tests/serialize/bug21957.phpt,v
retrieving revision 1.2
diff -u -r1.2 bug21957.phpt
--- ext/standard/tests/serialize/bug21957.phpt	30 Nov 2003 13:57:18 -0000	1.2
+++ ext/standard/tests/serialize/bug21957.phpt	6 Nov 2007 20:44:36 -0000
@@ -40,10 +40,15 @@
     int(2)
   }
 }
-a:2:{s:3:"one";s:3:"ABC";s:3:"two";N;}
+a:2:{s:3:"one";s:3:"ABC";s:3:"two";O:4:"test":2:{s:1:"a";i:7;s:1:"b";i:0;}}
 array(2) {
   ["one"]=>
   string(3) "ABC"
   ["two"]=>
-  NULL
+  object(test)#2 (2) {
+    ["a"]=>
+    int(7)
+    ["b"]=>
+    int(0)
+  }
 }

--MP_r/Ft77yW8YtYy/AOKc87Qo8--