ext/spl: Refactor debug handlers

Girgias · Girgias · commit 809410423f11 · 2024-06-15T01:33:09.000+01:00
Mainly to use zend_mangle_property_name() directly instead of spl_gen_private_prop_name()
diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c
@@ -24,11 +24,11 @@
 #include "zend_interfaces.h"
 #include "zend_exceptions.h"
 
-#include "spl_functions.h"
 #include "spl_iterators.h"
 #include "spl_array.h"
 #include "spl_array_arginfo.h"
 #include "spl_exceptions.h"
+#include "spl_functions.h" /* For spl_set_private_debug_info_property() */
 
 /* Defined later in the file */
 static zend_object_handlers spl_handler_ArrayIterator;
@@ -763,9 +763,6 @@ static HashTable *spl_array_get_properties_for(zend_object *object, zend_prop_pu
 
 static inline HashTable* spl_array_get_debug_info(zend_object *obj) /* {{{ */
 {
-	zval *storage;
-	zend_string *zname;
-	zend_class_entry *base;
 	spl_array_object *intern = spl_array_from_obj(obj);
 
 	if (!intern->std.properties) {
@@ -780,14 +777,13 @@ static inline HashTable* spl_array_get_debug_info(zend_object *obj) /* {{{ */
 		debug_info = zend_new_array(zend_hash_num_elements(intern->std.properties) + 1);
 		zend_hash_copy(debug_info, intern->std.properties, (copy_ctor_func_t) zval_add_ref);
 
-		storage = &intern->array;
+		zval *storage = &intern->array;
 		Z_TRY_ADDREF_P(storage);
 
-		base = obj->handlers == &spl_handler_ArrayIterator
+		const zend_class_entry *base_class_ce = instanceof_function(obj->ce, spl_ce_ArrayIterator)
 			? spl_ce_ArrayIterator : spl_ce_ArrayObject;
-		zname = spl_gen_private_prop_name(base, "storage", sizeof("storage")-1);
-		zend_symtable_update(debug_info, zname, storage);
-		zend_string_release_ex(zname, 0);
+
+		spl_set_private_debug_info_property(base_class_ce, "storage", strlen("storage"), debug_info, storage);
 
 		return debug_info;
 	}
diff --git a/ext/spl/spl_directory.c b/ext/spl/spl_directory.c
@@ -27,11 +27,11 @@
 #include "zend_exceptions.h"
 #include "zend_interfaces.h"
 
-#include "spl_functions.h" /* For spl_gen_private_prop_name() */
 #include "spl_iterators.h"
 #include "spl_directory.h"
 #include "spl_directory_arginfo.h"
 #include "spl_exceptions.h"
+#include "spl_functions.h" /* For spl_set_private_debug_info_property() */
 
 #define SPL_HAS_FLAG(flags, test_flag) ((flags & test_flag) ? 1 : 0)
 
@@ -600,84 +600,69 @@ static inline HashTable *spl_filesystem_object_get_debug_info(zend_object *objec
 {
 	spl_filesystem_object *intern = spl_filesystem_from_obj(object);
 	zval tmp;
-	HashTable *rv;
-	zend_string *pnstr;
-	zend_string *path;
-	char stmp[2];
+	HashTable *debug_info;
+	zend_string *path_name;
 
 	if (!intern->std.properties) {
 		rebuild_object_properties(&intern->std);
 	}
 
-	rv = zend_array_dup(intern->std.properties);
+	// TODO Do zend_new_array() + zend_hash_copy() trick?
+	debug_info = zend_array_dup(intern->std.properties);
 
-	pnstr = spl_gen_private_prop_name(spl_ce_SplFileInfo, "pathName", sizeof("pathName")-1);
-	path = spl_filesystem_object_get_pathname(intern);
-	if (path) {
-		ZVAL_STR_COPY(&tmp, path);
+	path_name = spl_filesystem_object_get_pathname(intern);
+	if (path_name) {
+		ZVAL_STR_COPY(&tmp, path_name);
 	} else {
 		ZVAL_EMPTY_STRING(&tmp);
 	}
-	zend_symtable_update(rv, pnstr, &tmp);
-	zend_string_release_ex(pnstr, 0);
+	/* IMPORTANT: Do not free path_name as spl_filesystem_object_get_pathname()
+	 * updates/sets the intern->file_name and returns the pointer to
+	 * intern->file_name which must remain allocated. */
+	spl_set_private_debug_info_property(spl_ce_SplFileInfo, "pathName", strlen("pathName"), debug_info, &tmp);
 
 	if (intern->file_name) {
-		zend_string *path;
-
-		pnstr = spl_gen_private_prop_name(spl_ce_SplFileInfo, "fileName", sizeof("fileName")-1);
-		path = spl_filesystem_object_get_path(intern);
-
+		zend_string *path = spl_filesystem_object_get_path(intern);
 		if (path && ZSTR_LEN(path) && ZSTR_LEN(path) < ZSTR_LEN(intern->file_name)) {
 			/* +1 to skip the trailing / of the path in the file name */
 			ZVAL_STRINGL(&tmp, ZSTR_VAL(intern->file_name) + ZSTR_LEN(path) + 1, ZSTR_LEN(intern->file_name) - (ZSTR_LEN(path) + 1));
 		} else {
 			ZVAL_STR_COPY(&tmp, intern->file_name);
 		}
-		zend_symtable_update(rv, pnstr, &tmp);
-		zend_string_release_ex(pnstr, /* persistent */ false);
 		if (path) {
 			zend_string_release_ex(path, /* persistent */ false);
 		}
+
+		spl_set_private_debug_info_property(spl_ce_SplFileInfo, "fileName", strlen("fileName"), debug_info, &tmp);
 	}
 	if (intern->type == SPL_FS_DIR) {
 #ifdef HAVE_GLOB
-		pnstr = spl_gen_private_prop_name(spl_ce_DirectoryIterator, "glob", sizeof("glob")-1);
-		if (php_stream_is(intern->u.dir.dirp ,&php_glob_stream_ops)) {
+		if (php_stream_is(intern->u.dir.dirp, &php_glob_stream_ops)) {
 			ZVAL_STR_COPY(&tmp, intern->path);
 		} else {
 			ZVAL_FALSE(&tmp);
 		}
-		zend_symtable_update(rv, pnstr, &tmp);
-		zend_string_release_ex(pnstr, 0);
+		spl_set_private_debug_info_property(spl_ce_DirectoryIterator, "glob", strlen("glob"), debug_info, &tmp);
 #endif
-		pnstr = spl_gen_private_prop_name(spl_ce_RecursiveDirectoryIterator, "subPathName", sizeof("subPathName")-1);
 		if (intern->u.dir.sub_path) {
 			ZVAL_STR_COPY(&tmp, intern->u.dir.sub_path);
 		} else {
 			ZVAL_EMPTY_STRING(&tmp);
 		}
-		zend_symtable_update(rv, pnstr, &tmp);
-		zend_string_release_ex(pnstr, 0);
+		spl_set_private_debug_info_property(spl_ce_RecursiveDirectoryIterator, "subPathName", strlen("subPathName"), debug_info, &tmp);
 	}
 	if (intern->type == SPL_FS_FILE) {
-		pnstr = spl_gen_private_prop_name(spl_ce_SplFileObject, "openMode", sizeof("openMode")-1);
 		ZVAL_STR_COPY(&tmp, intern->u.file.open_mode);
-		zend_symtable_update(rv, pnstr, &tmp);
-		zend_string_release_ex(pnstr, 0);
-		stmp[1] = '\0';
-		stmp[0] = intern->u.file.delimiter;
-		pnstr = spl_gen_private_prop_name(spl_ce_SplFileObject, "delimiter", sizeof("delimiter")-1);
-		ZVAL_STRINGL(&tmp, stmp, 1);
-		zend_symtable_update(rv, pnstr, &tmp);
-		zend_string_release_ex(pnstr, 0);
-		stmp[0] = intern->u.file.enclosure;
-		pnstr = spl_gen_private_prop_name(spl_ce_SplFileObject, "enclosure", sizeof("enclosure")-1);
-		ZVAL_STRINGL(&tmp, stmp, 1);
-		zend_symtable_update(rv, pnstr, &tmp);
-		zend_string_release_ex(pnstr, 0);
-	}
-
-	return rv;
+		spl_set_private_debug_info_property(spl_ce_SplFileObject, "openMode", strlen("openMode"), debug_info, &tmp);
+
+		ZVAL_STR(&tmp, ZSTR_CHAR((zend_uchar)intern->u.file.delimiter));
+		spl_set_private_debug_info_property(spl_ce_SplFileObject, "delimiter", strlen("delimiter"), debug_info, &tmp);
+
+		ZVAL_STR(&tmp, ZSTR_CHAR((zend_uchar)intern->u.file.enclosure));
+		spl_set_private_debug_info_property(spl_ce_SplFileObject, "enclosure", strlen("enclosure"), debug_info, &tmp);
+	}
+
+	return debug_info;
 }
 /* }}} */
 
diff --git a/ext/spl/spl_dllist.c b/ext/spl/spl_dllist.c
@@ -25,10 +25,10 @@
 
 #include "ext/standard/php_var.h"
 #include "zend_smart_str.h"
-#include "spl_functions.h" /* For spl_gen_private_prop_name() */
 #include "spl_dllist.h"
 #include "spl_dllist_arginfo.h"
 #include "spl_exceptions.h"
+#include "spl_functions.h" /* For spl_set_private_debug_info_property() */
 
 static zend_object_handlers spl_handler_SplDoublyLinkedList;
 PHPAPI zend_class_entry  *spl_ce_SplDoublyLinkedList;
@@ -424,41 +424,37 @@ static zend_result spl_dllist_object_count_elements(zend_object *object, zend_lo
 static inline HashTable* spl_dllist_object_get_debug_info(zend_object *obj) /* {{{{ */
 {
 	spl_dllist_object     *intern  = spl_dllist_from_obj(obj);
-	spl_ptr_llist_element *current = intern->llist->head, *next;
+	spl_ptr_llist_element *current = intern->llist->head;
 	zval tmp, dllist_array;
-	zend_string *pnstr;
-	int  i = 0;
 	HashTable *debug_info;
 
 	if (!intern->std.properties) {
 		rebuild_object_properties(&intern->std);
 	}
 
-	debug_info = zend_new_array(1);
+	/* +2 As we are adding 2 additional key-entries */
+	debug_info = zend_new_array(zend_hash_num_elements(intern->std.properties) + 2);
 	zend_hash_copy(debug_info, intern->std.properties, (copy_ctor_func_t) zval_add_ref);
 
-	pnstr = spl_gen_private_prop_name(spl_ce_SplDoublyLinkedList, "flags", sizeof("flags")-1);
 	ZVAL_LONG(&tmp, intern->flags);
-	zend_hash_add(debug_info, pnstr, &tmp);
-	zend_string_release_ex(pnstr, 0);
+	spl_set_private_debug_info_property(spl_ce_SplDoublyLinkedList, "flags", strlen("flags"), debug_info, &tmp);
 
 	array_init(&dllist_array);
 
+	zend_ulong index = 0;
 	while (current) {
-		next = current->next;
+		spl_ptr_llist_element *next = current->next;
 
-		add_index_zval(&dllist_array, i, &current->data);
+		add_index_zval(&dllist_array, index, &current->data);
 		if (Z_REFCOUNTED(current->data)) {
 			Z_ADDREF(current->data);
 		}
-		i++;
+		index++;
 
 		current = next;
 	}
 
-	pnstr = spl_gen_private_prop_name(spl_ce_SplDoublyLinkedList, "dllist", sizeof("dllist")-1);
-	zend_hash_add(debug_info, pnstr, &dllist_array);
-	zend_string_release_ex(pnstr, 0);
+	spl_set_private_debug_info_property(spl_ce_SplDoublyLinkedList, "dllist", strlen("dllist"), debug_info, &dllist_array);
 
 	return debug_info;
 }
diff --git a/ext/spl/spl_functions.c b/ext/spl/spl_functions.c
@@ -77,8 +77,21 @@ void spl_add_classes(zend_class_entry *pce, zval *list, bool sub, int allow, int
 }
 /* }}} */
 
-zend_string * spl_gen_private_prop_name(zend_class_entry *ce, char *prop_name, size_t prop_len) /* {{{ */
+void spl_set_private_debug_info_property(
+	const zend_class_entry *ce,
+	const char *property,
+	size_t property_len,
+	HashTable *debug_info,
+	zval *value
+)
 {
-	return zend_mangle_property_name(ZSTR_VAL(ce->name), ZSTR_LEN(ce->name), prop_name, prop_len, 0);
+	zend_string *mangled_named = zend_mangle_property_name(
+		ZSTR_VAL(ce->name),
+		ZSTR_LEN(ce->name),
+		property,
+		property_len,
+		/* persistent */ false
+	);
+	zend_hash_update(debug_info, mangled_named, value);
+	zend_string_release_ex(mangled_named, /* persistent */ false);
 }
-/* }}} */
diff --git a/ext/spl/spl_functions.h b/ext/spl/spl_functions.h
@@ -31,7 +31,6 @@ void spl_add_interfaces(zval * list, zend_class_entry * pce, int allow, int ce_f
 void spl_add_traits(zval * list, zend_class_entry * pce, int allow, int ce_flags);
 void spl_add_classes(zend_class_entry *pce, zval *list, bool sub, int allow, int ce_flags);
 
-/* caller must efree(return) */
-zend_string *spl_gen_private_prop_name(zend_class_entry *ce, char *prop_name, size_t prop_len);
+void spl_set_private_debug_info_property(const zend_class_entry *ce, const char *property, size_t property_len, HashTable *debug_info, zval *value);
 
 #endif /* PHP_FUNCTIONS_H */
diff --git a/ext/spl/spl_heap.c b/ext/spl/spl_heap.c
@@ -22,10 +22,10 @@
 #include "zend_interfaces.h"
 #include "zend_exceptions.h"
 
-#include "spl_functions.h"
 #include "spl_heap.h"
 #include "spl_heap_arginfo.h"
 #include "spl_exceptions.h"
+#include "spl_functions.h" /* For spl_set_private_debug_info_property() */
 
 #define PTR_HEAP_BLOCK_SIZE 64
 
@@ -503,33 +503,28 @@ static zend_result spl_heap_object_count_elements(zend_object *object, zend_long
 }
 /* }}} */
 
-static inline HashTable* spl_heap_object_get_debug_info(zend_class_entry *ce, zend_object *obj) { /* {{{ */
+static inline HashTable* spl_heap_object_get_debug_info(const zend_class_entry *ce, zend_object *obj) { /* {{{ */
 	spl_heap_object *intern = spl_heap_from_obj(obj);
 	zval tmp, heap_array;
-	zend_string *pnstr;
 	HashTable *debug_info;
-	int  i;
 
 	if (!intern->std.properties) {
 		rebuild_object_properties(&intern->std);
 	}
 
-	debug_info = zend_new_array(zend_hash_num_elements(intern->std.properties) + 1);
+	/* +3 As we are adding 3 additional key-entries */
+	debug_info = zend_new_array(zend_hash_num_elements(intern->std.properties) + 3);
 	zend_hash_copy(debug_info, intern->std.properties, (copy_ctor_func_t) zval_add_ref);
 
-	pnstr = spl_gen_private_prop_name(ce, "flags", sizeof("flags")-1);
 	ZVAL_LONG(&tmp, intern->flags);
-	zend_hash_update(debug_info, pnstr, &tmp);
-	zend_string_release_ex(pnstr, 0);
+	spl_set_private_debug_info_property(ce, "flags", strlen("flags"), debug_info, &tmp);
 
-	pnstr = spl_gen_private_prop_name(ce, "isCorrupted", sizeof("isCorrupted")-1);
 	ZVAL_BOOL(&tmp, intern->heap->flags&SPL_HEAP_CORRUPTED);
-	zend_hash_update(debug_info, pnstr, &tmp);
-	zend_string_release_ex(pnstr, 0);
+	spl_set_private_debug_info_property(ce, "isCorrupted", strlen("isCorrupted"), debug_info, &tmp);
 
 	array_init(&heap_array);
 
-	for (i = 0; i < intern->heap->count; ++i) {
+	for (zend_ulong i = 0; i < intern->heap->count; ++i) {
 		if (ce == spl_ce_SplPriorityQueue) {
 			spl_pqueue_elem *pq_elem = spl_heap_elem(intern->heap, i);
 			zval elem;
@@ -542,9 +537,7 @@ static inline HashTable* spl_heap_object_get_debug_info(zend_class_entry *ce, ze
 		}
 	}
 
-	pnstr = spl_gen_private_prop_name(ce, "heap", sizeof("heap")-1);
-	zend_hash_update(debug_info, pnstr, &heap_array);
-	zend_string_release_ex(pnstr, 0);
+	spl_set_private_debug_info_property(ce, "heap", strlen("heap"), debug_info, &heap_array);
 
 	return debug_info;
 }
diff --git a/ext/spl/spl_observer.c b/ext/spl/spl_observer.c
@@ -27,11 +27,11 @@
 #include "zend_exceptions.h"
 
 #include "php_spl.h" /* For php_spl_object_hash() */
-#include "spl_functions.h" /* For spl_gen_private_prop_name() */
 #include "spl_observer.h"
 #include "spl_observer_arginfo.h"
 #include "spl_iterators.h"
 #include "spl_exceptions.h"
+#include "spl_functions.h" /* For spl_set_private_debug_info_property() */
 
 PHPAPI zend_class_entry     *spl_ce_SplObserver;
 PHPAPI zend_class_entry     *spl_ce_SplSubject;
@@ -321,7 +321,6 @@ static inline HashTable* spl_object_storage_debug_info(zend_object *obj) /* {{{
 	spl_SplObjectStorageElement *element;
 	HashTable *props;
 	zval tmp, storage;
-	zend_string *zname;
 	HashTable *debug_info;
 
 	props = obj->handlers->get_properties(obj);
@@ -343,9 +342,7 @@ static inline HashTable* spl_object_storage_debug_info(zend_object *obj) /* {{{
 		zend_hash_next_index_insert(Z_ARRVAL(storage), &tmp);
 	} ZEND_HASH_FOREACH_END();
 
-	zname = spl_gen_private_prop_name(spl_ce_SplObjectStorage, "storage", sizeof("storage")-1);
-	zend_symtable_update(debug_info, zname, &storage);
-	zend_string_release_ex(zname, 0);
+	spl_set_private_debug_info_property(spl_ce_SplObjectStorage, "storage", strlen("storage"), debug_info, &storage);
 
 	return debug_info;
 }
