Skip to content

Commit 59a0d00

Browse files
ndosschendossche
committed
Avoid string duplications in dom iterators
1 parent 4c3aeec commit 59a0d00

File tree

5 files changed

+45
-38
lines changed

5 files changed

+45
-38
lines changed

‎ext/dom/documenttype.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ zend_result dom_documenttype_entities_read(dom_object *obj, zval *retval)
5252
xmlHashTable *entityht = (xmlHashTable *) dtdptr->entities;
5353

5454
dom_object *intern = Z_DOMOBJ_P(retval);
55-
dom_namednode_iter(obj, XML_ENTITY_NODE, intern, entityht, NULL, 0, NULL, 0);
55+
dom_namednode_iter(obj, XML_ENTITY_NODE, intern, entityht, NULL, NULL);
5656

5757
return SUCCESS;
5858
}
@@ -73,7 +73,7 @@ zend_result dom_documenttype_notations_read(dom_object *obj, zval *retval)
7373
xmlHashTable *notationht = (xmlHashTable *) dtdptr->notations;
7474

7575
dom_object *intern = Z_DOMOBJ_P(retval);
76-
dom_namednode_iter(obj, XML_NOTATION_NODE, intern, notationht, NULL, 0, NULL, 0);
76+
dom_namednode_iter(obj, XML_NOTATION_NODE, intern, notationht, NULL, NULL);
7777

7878
return SUCCESS;
7979
}

‎ext/dom/element.c‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -811,15 +811,14 @@ Modern spec URL: https://dom.spec.whatwg.org/#concept-getelementsbytagname
811811
*/
812812
static void dom_element_get_elements_by_tag_name(INTERNAL_FUNCTION_PARAMETERS, bool modern)
813813
{
814-
size_t name_len;
815814
dom_object *intern, *namednode;
816-
char *name;
815+
zend_string *name;
817816

818-
if (zend_parse_parameters(ZEND_NUM_ARGS(), "p", &name, &name_len) == FAILURE) {
817+
if (zend_parse_parameters(ZEND_NUM_ARGS(), "P", &name) == FAILURE) {
819818
RETURN_THROWS();
820819
}
821820

822-
if (name_len > INT_MAX) {
821+
if (ZSTR_LEN(name) > INT_MAX) {
823822
zend_argument_value_error(1, "is too long");
824823
RETURN_THROWS();
825824
}
@@ -832,7 +831,7 @@ static void dom_element_get_elements_by_tag_name(INTERNAL_FUNCTION_PARAMETERS, b
832831
php_dom_create_iterator(return_value, DOM_NODELIST, false);
833832
}
834833
namednode = Z_DOMOBJ_P(return_value);
835-
dom_namednode_iter(intern, 0, namednode, NULL, name, name_len, NULL, 0);
834+
dom_namednode_iter(intern, 0, namednode, NULL, name, NULL);
836835
}
837836

838837
PHP_METHOD(DOMElement, getElementsByTagName)
@@ -1239,20 +1238,23 @@ Since: DOM Level 2
12391238
*/
12401239
static void dom_element_get_elements_by_tag_name_ns(INTERNAL_FUNCTION_PARAMETERS, bool modern)
12411240
{
1242-
size_t uri_len, name_len;
12431241
dom_object *intern, *namednode;
1244-
char *uri, *name;
1242+
zend_string *uri, *name;
12451243

1246-
if (zend_parse_parameters(ZEND_NUM_ARGS(), "p!p", &uri, &uri_len, &name, &name_len) == FAILURE) {
1244+
if (zend_parse_parameters(ZEND_NUM_ARGS(), "P!P", &uri, &name) == FAILURE) {
12471245
RETURN_THROWS();
12481246
}
12491247

1250-
if (uri_len > INT_MAX) {
1248+
if (!uri) {
1249+
uri = ZSTR_EMPTY_ALLOC();
1250+
}
1251+
1252+
if (ZSTR_LEN(uri) > INT_MAX) {
12511253
zend_argument_value_error(1, "is too long");
12521254
RETURN_THROWS();
12531255
}
12541256

1255-
if (name_len > INT_MAX) {
1257+
if (ZSTR_LEN(name) > INT_MAX) {
12561258
zend_argument_value_error(2, "is too long");
12571259
RETURN_THROWS();
12581260
}
@@ -1265,7 +1267,7 @@ static void dom_element_get_elements_by_tag_name_ns(INTERNAL_FUNCTION_PARAMETERS
12651267
php_dom_create_iterator(return_value, DOM_NODELIST, false);
12661268
}
12671269
namednode = Z_DOMOBJ_P(return_value);
1268-
dom_namednode_iter(intern, 0, namednode, NULL, name, name_len, uri ? uri : "", uri_len);
1270+
dom_namednode_iter(intern, 0, namednode, NULL, name, uri);
12691271
}
12701272

12711273
PHP_METHOD(DOMElement, getElementsByTagNameNS)

‎ext/dom/node.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,7 @@ zend_result dom_node_child_nodes_read(dom_object *obj, zval *retval)
283283

284284
php_dom_create_iterator(retval, DOM_NODELIST, php_dom_follow_spec_intern(obj));
285285
dom_object *intern = Z_DOMOBJ_P(retval);
286-
dom_namednode_iter(obj, XML_ELEMENT_NODE, intern, NULL, NULL, 0, NULL, 0);
286+
dom_namednode_iter(obj, XML_ELEMENT_NODE, intern, NULL, NULL, NULL);
287287

288288
return SUCCESS;
289289
}
@@ -417,7 +417,7 @@ zend_result dom_node_attributes_read(dom_object *obj, zval *retval)
417417
if (nodep->type == XML_ELEMENT_NODE) {
418418
php_dom_create_iterator(retval, DOM_NAMEDNODEMAP, php_dom_follow_spec_intern(obj));
419419
dom_object *intern = Z_DOMOBJ_P(retval);
420-
dom_namednode_iter(obj, XML_ATTRIBUTE_NODE, intern, NULL, NULL, 0, NULL, 0);
420+
dom_namednode_iter(obj, XML_ATTRIBUTE_NODE, intern, NULL, NULL, NULL);
421421
} else {
422422
ZVAL_NULL(retval);
423423
}

‎ext/dom/php_dom.c‎

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1452,7 +1452,7 @@ void dom_objects_free_storage(zend_object *object)
14521452
}
14531453
/* }}} */
14541454

1455-
void dom_namednode_iter(dom_object *basenode, int ntype, dom_object *intern, xmlHashTablePtr ht, const char *local, size_t local_len, const char *ns, size_t ns_len) /* {{{ */
1455+
void dom_namednode_iter(dom_object *basenode, int ntype, dom_object *intern, xmlHashTablePtr ht, zend_string *local, zend_string *ns) /* {{{ */
14561456
{
14571457
dom_nnodemap_object *mapptr = (dom_nnodemap_object *) intern->ptr;
14581458

@@ -1473,24 +1473,23 @@ void dom_namednode_iter(dom_object *basenode, int ntype, dom_object *intern, xml
14731473
const xmlChar* tmp;
14741474

14751475
if (local) {
1476-
int len = (int) local_len;
1476+
int len = (int) ZSTR_LEN(local);
14771477
if (doc != NULL && (tmp = xmlDictExists(doc->dict, (const xmlChar *)local, len)) != NULL) {
14781478
mapptr->local = BAD_CAST tmp;
14791479
} else {
1480-
mapptr->local = xmlCharStrndup(local, len);
1481-
mapptr->free_local = true;
1480+
mapptr->local = BAD_CAST ZSTR_VAL(zend_string_copy(local));
1481+
mapptr->release_local = true;
14821482
}
1483-
mapptr->local_lower = BAD_CAST estrdup(local);
1484-
zend_str_tolower((char *) mapptr->local_lower, len);
1483+
mapptr->local_lower = zend_string_tolower(local);
14851484
}
14861485

14871486
if (ns) {
1488-
int len = (int) ns_len;
1487+
int len = (int) ZSTR_LEN(ns);
14891488
if (doc != NULL && (tmp = xmlDictExists(doc->dict, (const xmlChar *)ns, len)) != NULL) {
14901489
mapptr->ns = BAD_CAST tmp;
14911490
} else {
1492-
mapptr->ns = xmlCharStrndup(ns, len);
1493-
mapptr->free_ns = true;
1491+
mapptr->ns = BAD_CAST ZSTR_VAL(zend_string_copy(ns));
1492+
mapptr->release_ns = true;
14941493
}
14951494
}
14961495
}
@@ -1561,6 +1560,11 @@ zend_object *dom_xpath_objects_new(zend_class_entry *class_type)
15611560

15621561
#endif
15631562

1563+
/* The char pointer MUST refer to the char* of a zend_string struct */
1564+
static void dom_zend_string_release_from_char_pointer(xmlChar *ptr) {
1565+
zend_string_release((zend_string*) (ptr - XtOffsetOf(zend_string, val)));
1566+
}
1567+
15641568
void dom_nnodemap_objects_free_storage(zend_object *object) /* {{{ */
15651569
{
15661570
dom_object *intern = php_dom_obj_from_obj(object);
@@ -1570,14 +1574,14 @@ void dom_nnodemap_objects_free_storage(zend_object *object) /* {{{ */
15701574
if (objmap->cached_obj && GC_DELREF(&objmap->cached_obj->std) == 0) {
15711575
zend_objects_store_del(&objmap->cached_obj->std);
15721576
}
1573-
if (objmap->free_local) {
1574-
xmlFree(objmap->local);
1577+
if (objmap->release_local) {
1578+
dom_zend_string_release_from_char_pointer(objmap->local);
15751579
}
1576-
if (objmap->free_ns) {
1577-
xmlFree(objmap->ns);
1580+
if (objmap->release_ns) {
1581+
dom_zend_string_release_from_char_pointer(objmap->ns);
15781582
}
15791583
if (objmap->local_lower) {
1580-
efree(objmap->local_lower);
1584+
zend_string_release(objmap->local_lower);
15811585
}
15821586
if (!Z_ISUNDEF(objmap->baseobj_zv)) {
15831587
zval_ptr_dtor(&objmap->baseobj_zv);
@@ -1607,9 +1611,9 @@ zend_object *dom_nnodemap_objects_new(zend_class_entry *class_type)
16071611
objmap->ht = NULL;
16081612
objmap->local = NULL;
16091613
objmap->local_lower = NULL;
1610-
objmap->free_local = false;
1614+
objmap->release_local = false;
16111615
objmap->ns = NULL;
1612-
objmap->free_ns = false;
1616+
objmap->release_ns = false;
16131617
objmap->cache_tag.modification_nr = 0;
16141618
objmap->cached_length = -1;
16151619
objmap->cached_obj = NULL;
@@ -1865,7 +1869,7 @@ static bool dom_match_qualified_name_for_tag_name_equality(const xmlChar *local,
18651869
return dom_match_qualified_name_according_to_spec(local_to_use, nodep);
18661870
}
18671871

1868-
xmlNode *dom_get_elements_by_tag_name_ns_raw(xmlNodePtr basep, xmlNodePtr nodep, xmlChar *ns, xmlChar *local, xmlChar *local_lower, zend_long *cur, zend_long index) /* {{{ */
1872+
xmlNode *dom_get_elements_by_tag_name_ns_raw(xmlNodePtr basep, xmlNodePtr nodep, xmlChar *ns, xmlChar *local, zend_string *local_lower, zend_long *cur, zend_long index) /* {{{ */
18691873
{
18701874
/* Can happen with detached document */
18711875
if (UNEXPECTED(nodep == NULL)) {
@@ -1884,7 +1888,7 @@ xmlNode *dom_get_elements_by_tag_name_ns_raw(xmlNodePtr basep, xmlNodePtr nodep,
18841888

18851889
while (*cur <= index) {
18861890
if (nodep->type == XML_ELEMENT_NODE) {
1887-
if (local_match_any || dom_match_qualified_name_for_tag_name_equality(local, local_lower, nodep, match_qname)) {
1891+
if (local_match_any || dom_match_qualified_name_for_tag_name_equality(local, BAD_CAST ZSTR_VAL(local_lower), nodep, match_qname)) {
18881892
if (ns_match_any || (ns[0] == '\0' && nodep->ns == NULL) || (nodep->ns != NULL && xmlStrEqual(nodep->ns->href, ns))) {
18891893
if (*cur == index) {
18901894
ret = nodep;

‎ext/dom/php_dom.h‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -83,14 +83,15 @@ typedef struct dom_nnodemap_object {
8383
int nodetype;
8484
int cached_length;
8585
xmlHashTable *ht;
86-
xmlChar *local, *local_lower;
86+
xmlChar *local;
87+
zend_string *local_lower;
8788
xmlChar *ns;
8889
php_libxml_cache_tag cache_tag;
8990
dom_object *cached_obj;
9091
zend_long cached_obj_index;
9192
xmlDictPtr dict;
92-
bool free_local : 1;
93-
bool free_ns : 1;
93+
bool release_local : 1;
94+
bool release_ns : 1;
9495
} dom_nnodemap_object;
9596

9697
typedef struct {
@@ -144,14 +145,14 @@ void dom_reconcile_ns_list(xmlDocPtr doc, xmlNodePtr nodep, xmlNodePtr last);
144145
xmlNsPtr dom_get_nsdecl(xmlNode *node, xmlChar *localName);
145146
void php_dom_normalize_legacy(xmlNodePtr nodep);
146147
void php_dom_normalize_modern(xmlNodePtr nodep);
147-
xmlNode *dom_get_elements_by_tag_name_ns_raw(xmlNodePtr basep, xmlNodePtr nodep, xmlChar *ns, xmlChar *local, xmlChar *local_lower, zend_long *cur, zend_long index);
148+
xmlNode *dom_get_elements_by_tag_name_ns_raw(xmlNodePtr basep, xmlNodePtr nodep, xmlChar *ns, xmlChar *local, zend_string *local_lower, zend_long *cur, zend_long index);
148149
void php_dom_create_implementation(zval *retval, bool modern);
149150
int dom_hierarchy(xmlNodePtr parent, xmlNodePtr child);
150151
bool dom_has_feature(zend_string *feature, zend_string *version);
151152
bool dom_node_is_read_only(const xmlNode *node);
152153
bool dom_node_children_valid(const xmlNode *node);
153154
void php_dom_create_iterator(zval *return_value, dom_iterator_type iterator_type, bool modern);
154-
void dom_namednode_iter(dom_object *basenode, int ntype, dom_object *intern, xmlHashTablePtr ht, const char *local, size_t local_len, const char *ns, size_t ns_len);
155+
void dom_namednode_iter(dom_object *basenode, int ntype, dom_object *intern, xmlHashTablePtr ht, zend_string *local, zend_string *ns);
155156
xmlNodePtr create_notation(const xmlChar *name, const xmlChar *ExternalID, const xmlChar *SystemID);
156157
xmlNode *php_dom_libxml_hash_iter(dom_nnodemap_object *objmap, int index);
157158
zend_object_iterator *php_dom_get_iterator(zend_class_entry *ce, zval *object, int by_ref);

0 commit comments

Comments
 (0)