Refactor implementation of DOM nodelists, named maps, and iterators

The code was really messy with lots of checks and inconsistencies.
This splits everything up into different functions and now everything is
relayed to a handler vtable.

Author: ndossche

ext/dom/config.m4

@@ -33,6 +33,7 @@ if test "$PHP_DOM" != "no"; then
         node.c
         nodelist.c
         notation.c
+        obj_map.c
         parentnode/css_selectors.c
         parentnode/tree.c
         php_dom.c

ext/dom/config.w32

@@ -15,7 +15,7 @@ if (PHP_DOM == "yes") {
 			entity.c nodelist.c html_collection.c text.c comment.c \
 			entityreference.c \
 			token_list.c \
-			notation.c xpath.c dom_iterators.c \
+			notation.c obj_map.c xpath.c dom_iterators.c \
 			namednodemap.c xpath_callbacks.c", null, "/I ext/lexbor");
 
 		ADD_EXTENSION_DEP('dom', 'lexbor');

ext/dom/documenttype.c

@@ -53,7 +53,7 @@ zend_result dom_documenttype_entities_read(dom_object *obj, zval *retval)
 	xmlHashTable *entityht = (xmlHashTable *) dtdptr->entities;
 
 	dom_object *intern = Z_DOMOBJ_P(retval);
-	dom_namednode_iter(obj, XML_ENTITY_NODE, intern, entityht, NULL, NULL);
+	dom_namednode_iter(obj, intern, entityht, NULL, NULL, &php_dom_obj_map_entities);
 
 	return SUCCESS;
 }
@@ -74,7 +74,7 @@ zend_result dom_documenttype_notations_read(dom_object *obj, zval *retval)
 	xmlHashTable *notationht = (xmlHashTable *) dtdptr->notations;
 
 	dom_object *intern = Z_DOMOBJ_P(retval);
-	dom_namednode_iter(obj, XML_NOTATION_NODE, intern, notationht, NULL, NULL);
+	dom_namednode_iter(obj, intern, notationht, NULL, NULL, &php_dom_obj_map_notations);
 
 	return SUCCESS;
 }

ext/dom/dom_iterators.c

@@ -68,7 +68,7 @@ xmlNodePtr create_notation(const xmlChar *name, const xmlChar *ExternalID, const
 }
 /* }}} */
 
-static xmlNode *php_dom_libxml_hash_iter_ex(xmlHashTable *ht, int index)
+xmlNodePtr php_dom_libxml_hash_iter(xmlHashTable *ht, int index)
 {
 	int htsize;
 
@@ -84,18 +84,6 @@ static xmlNode *php_dom_libxml_hash_iter_ex(xmlHashTable *ht, int index)
 	}
 }
 
-xmlNode *php_dom_libxml_hash_iter(dom_nnodemap_object *objmap, int index)
-{
-	xmlNode *curnode = php_dom_libxml_hash_iter_ex(objmap->ht, index);
-
-	if (curnode != NULL && objmap->nodetype != XML_ENTITY_NODE) {
-		xmlNotation *notation = (xmlNotation *) curnode;
-		curnode = create_notation(notation->name, notation->PublicID, notation->SystemID);
-	}
-
-	return curnode;
-}
-
 static void php_dom_iterator_dtor(zend_object_iterator *iter) /* {{{ */
 {
 	php_dom_iterator *iterator = (php_dom_iterator *)iter;
@@ -109,7 +97,7 @@ static zend_result php_dom_iterator_valid(zend_object_iterator *iter) /* {{{ */
 {
 	php_dom_iterator *iterator = (php_dom_iterator *)iter;
 
-	if (Z_TYPE(iterator->curobj) != IS_UNDEF) {
+	if (!Z_ISNULL(iterator->curobj)) {
 		return SUCCESS;
 	} else {
 		return FAILURE;
@@ -120,7 +108,7 @@ static zend_result php_dom_iterator_valid(zend_object_iterator *iter) /* {{{ */
 zval *php_dom_iterator_current_data(zend_object_iterator *iter) /* {{{ */
 {
 	php_dom_iterator *iterator = (php_dom_iterator *)iter;
-	return Z_ISUNDEF(iterator->curobj) ? NULL : &iterator->curobj;
+	return Z_ISNULL(iterator->curobj) ? NULL : &iterator->curobj;
 }
 /* }}} */
 
@@ -131,14 +119,14 @@ static void php_dom_iterator_current_key(zend_object_iterator *iter, zval *key)
 
 	/* Only dtd named node maps, i.e. the ones based on a libxml hash table or attribute collections,
 	 * are keyed by the name because in that case the name is unique. */
-	if (!objmap->ht && objmap->nodetype != XML_ATTRIBUTE_NODE) {
+	if (objmap->handler->nameless) {
 		ZVAL_LONG(key, iterator->index);
 	} else {
 		dom_object *intern = Z_DOMOBJ_P(&iterator->curobj);
 
-		if (intern != NULL && intern->ptr != NULL) {
+		if (intern->ptr != NULL) {
 			xmlNodePtr curnode = ((php_libxml_node_ptr *)intern->ptr)->node;
-			if (objmap->nodetype == XML_ATTRIBUTE_NODE && php_dom_follow_spec_intern(intern)) {
+			if (curnode->type == XML_ATTRIBUTE_NODE && php_dom_follow_spec_intern(intern)) {
 				ZVAL_NEW_STR(key, dom_node_get_node_name_attribute_or_element(curnode, false));
 			} else {
 				ZVAL_STRINGL(key, (const char *) curnode->name, xmlStrlen(curnode->name));
@@ -150,99 +138,36 @@ static void php_dom_iterator_current_key(zend_object_iterator *iter, zval *key)
 }
 /* }}} */
 
-static xmlNodePtr dom_fetch_first_iteration_item(dom_nnodemap_object *objmap)
-{
-	xmlNodePtr basep = dom_object_get_node(objmap->baseobj);
-	if (!basep) {
-		return NULL;
-	}
-	if (objmap->nodetype == XML_ATTRIBUTE_NODE || objmap->nodetype == XML_ELEMENT_NODE) {
-		if (objmap->nodetype == XML_ATTRIBUTE_NODE) {
-			return (xmlNodePtr) basep->properties;
-		} else {
-			return dom_nodelist_iter_start_first_child(basep);
-		}
-	} else {
-		zend_long curindex = 0;
-		xmlNodePtr nodep = php_dom_first_child_of_container_node(basep);
-		return dom_get_elements_by_tag_name_ns_raw(
-			basep, nodep, objmap->ns, objmap->local, objmap->local_lower, &curindex, 0);
-	}
-}
-
 static void php_dom_iterator_move_forward(zend_object_iterator *iter) /* {{{ */
 {
-	xmlNodePtr curnode = NULL;
-
 	php_dom_iterator *iterator = (php_dom_iterator *)iter;
-	if (Z_ISUNDEF(iterator->curobj)) {
+	if (Z_ISNULL(iterator->curobj)) {
 		return;
 	}
 
 	iterator->index++;
+	zval garbage;
+	ZVAL_COPY_VALUE(&garbage, &iterator->curobj);
+	ZVAL_NULL(&iterator->curobj);
 
 	dom_object *intern = Z_DOMOBJ_P(&iterator->curobj);
 	dom_nnodemap_object *objmap = php_dom_iterator_get_nnmap(iterator);
 
-	if (intern != NULL && intern->ptr != NULL) {
-		if (objmap->nodetype != XML_ENTITY_NODE &&
-			objmap->nodetype != XML_NOTATION_NODE) {
-			if (objmap->nodetype == DOM_NODESET) {
-				HashTable *nodeht = Z_ARRVAL_P(&objmap->baseobj_zv);
-				zval *entry = zend_hash_index_find(nodeht, iterator->index);
-				if (entry) {
-					zval_ptr_dtor(&iterator->curobj);
-					ZVAL_COPY(&iterator->curobj, entry);
-					return;
-				}
-			} else {
-				if (objmap->nodetype == XML_ATTRIBUTE_NODE ||
-					objmap->nodetype == XML_ELEMENT_NODE) {
-
-					/* Note: keep legacy behaviour for non-spec mode. */
-					if (php_dom_follow_spec_intern(intern) && php_dom_is_cache_tag_stale_from_doc_ptr(&iterator->cache_tag, intern->document)) {
-						php_dom_mark_cache_tag_up_to_date_from_doc_ref(&iterator->cache_tag, intern->document);
-						curnode = dom_fetch_first_iteration_item(objmap);
-						zend_ulong index = 0;
-						while (curnode != NULL && index++ < iterator->index) {
-							curnode = curnode->next;
-						}
-					} else {
-						curnode = (xmlNodePtr)((php_libxml_node_ptr *)intern->ptr)->node;
-						curnode = curnode->next;
-					}
-				} else {
-					/* The collection is live, we nav the tree from the base object if we cannot
-					 * use the cache to restart from the last point. */
-					xmlNodePtr basenode = dom_object_get_node(objmap->baseobj);
-
-					/* We have a strong reference to the base node via baseobj_zv, this cannot become NULL */
-					ZEND_ASSERT(basenode != NULL);
-
-					zend_long previndex;
-					if (php_dom_is_cache_tag_stale_from_node(&iterator->cache_tag, basenode)) {
-						php_dom_mark_cache_tag_up_to_date_from_node(&iterator->cache_tag, basenode);
-						previndex = 0;
-						curnode = php_dom_first_child_of_container_node(basenode);
-					} else {
-						previndex = iterator->index - 1;
-						curnode = (xmlNodePtr)((php_libxml_node_ptr *)intern->ptr)->node;
-					}
-					curnode = dom_get_elements_by_tag_name_ns_raw(
-						basenode, curnode, objmap->ns, objmap->local, objmap->local_lower, &previndex, iterator->index);
-				}
+	if (intern->ptr != NULL) {
+		/* Note: keep legacy behaviour for non-spec mode. */
+		/* TODO: make this prettier */
+		if (!php_dom_follow_spec_intern(intern) && (objmap->handler == &php_dom_obj_map_attributes || objmap->handler == &php_dom_obj_map_child_nodes)) {
+			xmlNodePtr curnode = ((php_libxml_node_ptr *) intern->ptr)->node;
+			curnode = curnode->next;
+			if (curnode) {
+				php_dom_create_object(curnode, &iterator->curobj, objmap->baseobj);
 			}
 		} else {
-			curnode = php_dom_libxml_hash_iter(objmap, iterator->index);
+			objmap->handler->get_item(objmap, (zend_long) iterator->index, &iterator->curobj);
 		}
 	}
 
-	zval_ptr_dtor(&iterator->curobj);
-	ZVAL_UNDEF(&iterator->curobj);
-
-	if (curnode) {
-		php_dom_create_object(curnode, &iterator->curobj, objmap->baseobj);
-	}
+	zval_ptr_dtor(&garbage);
 }
 /* }}} */
 
@@ -261,7 +186,6 @@ zend_object_iterator *php_dom_get_iterator(zend_class_entry *ce, zval *object, i
 {
 	dom_object *intern;
 	dom_nnodemap_object *objmap;
-	xmlNodePtr curnode=NULL;
 	php_dom_iterator *iterator;
 
 	if (by_ref) {
@@ -277,26 +201,7 @@ zend_object_iterator *php_dom_get_iterator(zend_class_entry *ce, zval *object, i
 
 	intern = Z_DOMOBJ_P(object);
 	objmap = (dom_nnodemap_object *)intern->ptr;
-	if (objmap != NULL) {
-		if (objmap->nodetype != XML_ENTITY_NODE &&
-			objmap->nodetype != XML_NOTATION_NODE) {
-			if (objmap->nodetype == DOM_NODESET) {
-				HashTable *nodeht = Z_ARRVAL_P(&objmap->baseobj_zv);
-				zval *entry = zend_hash_index_find(nodeht, 0);
-				if (entry) {
-					ZVAL_COPY(&iterator->curobj, entry);
-				}
-			} else {
-				curnode = dom_fetch_first_iteration_item(objmap);
-			}
-		} else {
-			curnode = php_dom_libxml_hash_iter(objmap, 0);
-		}
-	}
-
-	if (curnode) {
-		php_dom_create_object(curnode, &iterator->curobj, objmap->baseobj);
-	}
+	objmap->handler->get_item(objmap, 0, &iterator->curobj);
 
 	return &iterator->intern;
 }

ext/dom/element.c

@@ -825,7 +825,7 @@ static void dom_element_get_elements_by_tag_name(INTERNAL_FUNCTION_PARAMETERS, z
 
 	object_init_ex(return_value, iter_ce);
 	namednode = Z_DOMOBJ_P(return_value);
-	dom_namednode_iter(intern, 0, namednode, NULL, name, NULL);
+	dom_namednode_iter(intern, namednode, NULL, name, NULL, &php_dom_obj_map_by_tag_name);
 }
 
 PHP_METHOD(DOMElement, getElementsByTagName)
@@ -1257,7 +1257,7 @@ static void dom_element_get_elements_by_tag_name_ns(INTERNAL_FUNCTION_PARAMETERS
 
 	object_init_ex(return_value, iter_ce);
 	namednode = Z_DOMOBJ_P(return_value);
-	dom_namednode_iter(intern, 0, namednode, NULL, name, uri);
+	dom_namednode_iter(intern, namednode, NULL, name, uri, &php_dom_obj_map_by_tag_name);
 }
 
 PHP_METHOD(DOMElement, getElementsByTagNameNS)