Skip to content

Some ext/simplexml cleanups #11360

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 4, 2023
Merged

Some ext/simplexml cleanups #11360

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
894e8ad
Remove dead code from sxe_get_element_by_name()
nielsdos Jun 2, 2023
1fd23da
Remove double class entry variable
nielsdos Jun 2, 2023
8dfefb1
Use xmlStrEqual() instead of !xmlStrCmp()
nielsdos Jun 2, 2023
713f64c
No need for the double name pointer
nielsdos Jun 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 39 additions & 55 deletions ext/simplexml/simplexml.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,12 @@
#include "zend_interfaces.h"
#include "ext/spl/spl_iterators.h"

zend_class_entry *sxe_class_entry = NULL;
PHP_SXE_API zend_class_entry *ce_SimpleXMLIterator;
PHP_SXE_API zend_class_entry *ce_SimpleXMLElement;

PHP_SXE_API zend_class_entry *sxe_get_element_class_entry(void) /* {{{ */
{
return sxe_class_entry;
return ce_SimpleXMLElement;
}
/* }}} */

Expand Down Expand Up @@ -101,7 +100,7 @@ static inline int match_ns(php_sxe_object *sxe, xmlNodePtr node, xmlChar *name,
return 1;
}

if (node->ns && !xmlStrcmp(prefix ? node->ns->prefix : node->ns->href, name)) {
if (node->ns && xmlStrEqual(prefix ? node->ns->prefix : node->ns->href, name)) {
return 1;
}

Expand All @@ -127,7 +126,7 @@ static xmlNodePtr sxe_get_element_by_offset(php_sxe_object *sxe, zend_long offse
SKIP_TEXT(node)
if (node->type == XML_ELEMENT_NODE && match_ns(sxe, node, sxe->iter.nsprefix, sxe->iter.isprefix)) {
if (sxe->iter.type == SXE_ITER_CHILD || (
sxe->iter.type == SXE_ITER_ELEMENT && !xmlStrcmp(node->name, sxe->iter.name))) {
sxe->iter.type == SXE_ITER_ELEMENT && xmlStrEqual(node->name, sxe->iter.name))) {
if (nodendx == offset) {
break;
}
Expand All @@ -151,7 +150,7 @@ static xmlNodePtr sxe_find_element_by_name(php_sxe_object *sxe, xmlNodePtr node,
while (node) {
SKIP_TEXT(node)
if (node->type == XML_ELEMENT_NODE && match_ns(sxe, node, sxe->iter.nsprefix, sxe->iter.isprefix)) {
if (!xmlStrcmp(node->name, name)) {
if (xmlStrEqual(node->name, name)) {
return node;
}
}
Expand All @@ -161,11 +160,10 @@ static xmlNodePtr sxe_find_element_by_name(php_sxe_object *sxe, xmlNodePtr node,
return NULL;
} /* }}} */

static xmlNodePtr sxe_get_element_by_name(php_sxe_object *sxe, xmlNodePtr node, char **name, SXE_ITER *type) /* {{{ */
static xmlNodePtr sxe_get_element_by_name(php_sxe_object *sxe, xmlNodePtr node, char *name, SXE_ITER *type) /* {{{ */
{
int orgtype;
xmlNodePtr orgnode = node;
xmlNodePtr retnode = NULL;

if (sxe->iter.type != SXE_ITER_ATTRLIST)
{
Expand All @@ -188,26 +186,15 @@ static xmlNodePtr sxe_get_element_by_name(php_sxe_object *sxe, xmlNodePtr node,
while (node) {
SKIP_TEXT(node)
if (node->type == XML_ELEMENT_NODE && match_ns(sxe, node, sxe->iter.nsprefix, sxe->iter.isprefix)) {
if (!xmlStrcmp(node->name, (xmlChar *)*name)) {
if (1||retnode)
{
*type = SXE_ITER_ELEMENT;
return orgnode;
}
retnode = node;
if (xmlStrEqual(node->name, (xmlChar *)name)) {
*type = SXE_ITER_ELEMENT;
return orgnode;
}
}
next_iter:
node = node->next;
}

if (retnode)
{
*type = SXE_ITER_NONE;
*name = NULL;
return retnode;
}

return NULL;
}
/* }}} */
Expand Down Expand Up @@ -281,7 +268,7 @@ static zval *sxe_prop_dim_read(zend_object *object, zval *member, bool elements,
if (Z_TYPE_P(member) != IS_LONG || sxe->iter.type == SXE_ITER_ATTRLIST) {
if (Z_TYPE_P(member) == IS_LONG) {
while (attr && nodendx <= Z_LVAL_P(member)) {
if ((!test || !xmlStrcmp(attr->name, sxe->iter.name)) && match_ns(sxe, (xmlNodePtr) attr, sxe->iter.nsprefix, sxe->iter.isprefix)) {
if ((!test || xmlStrEqual(attr->name, sxe->iter.name)) && match_ns(sxe, (xmlNodePtr) attr, sxe->iter.nsprefix, sxe->iter.isprefix)) {
if (nodendx == Z_LVAL_P(member)) {
_node_as_zval(sxe, (xmlNodePtr) attr, rv, SXE_ITER_NONE, NULL, sxe->iter.nsprefix, sxe->iter.isprefix);
break;
Expand All @@ -292,7 +279,7 @@ static zval *sxe_prop_dim_read(zend_object *object, zval *member, bool elements,
}
} else {
while (attr) {
if ((!test || !xmlStrcmp(attr->name, sxe->iter.name)) && !xmlStrcmp(attr->name, (xmlChar *)name) && match_ns(sxe, (xmlNodePtr) attr, sxe->iter.nsprefix, sxe->iter.isprefix)) {
if ((!test || xmlStrEqual(attr->name, sxe->iter.name)) && xmlStrEqual(attr->name, (xmlChar *)name) && match_ns(sxe, (xmlNodePtr) attr, sxe->iter.nsprefix, sxe->iter.isprefix)) {
_node_as_zval(sxe, (xmlNodePtr) attr, rv, SXE_ITER_NONE, NULL, sxe->iter.nsprefix, sxe->iter.isprefix);
break;
}
Expand Down Expand Up @@ -483,7 +470,7 @@ static zval *sxe_prop_dim_write(zend_object *object, zval *member, zval *value,
value_str = zval_get_string(value);
break;
case IS_OBJECT:
if (Z_OBJCE_P(value) == sxe_class_entry) {
if (Z_OBJCE_P(value) == ce_SimpleXMLElement) {
zval zval_copy;
if (sxe_object_cast_ex(Z_OBJ_P(value), &zval_copy, IS_STRING) == FAILURE) {
zend_throw_error(NULL, "Unable to cast node to string");
Expand All @@ -507,7 +494,7 @@ static zval *sxe_prop_dim_write(zend_object *object, zval *member, zval *value,
if (attribs) {
if (Z_TYPE_P(member) == IS_LONG) {
while (attr && nodendx <= Z_LVAL_P(member)) {
if ((!test || !xmlStrcmp(attr->name, sxe->iter.name)) && match_ns(sxe, (xmlNodePtr) attr, sxe->iter.nsprefix, sxe->iter.isprefix)) {
if ((!test || xmlStrEqual(attr->name, sxe->iter.name)) && match_ns(sxe, (xmlNodePtr) attr, sxe->iter.nsprefix, sxe->iter.isprefix)) {
if (nodendx == Z_LVAL_P(member)) {
is_attr = 1;
++counter;
Expand All @@ -519,7 +506,7 @@ static zval *sxe_prop_dim_write(zend_object *object, zval *member, zval *value,
}
} else {
while (attr) {
if ((!test || !xmlStrcmp(attr->name, sxe->iter.name)) && !xmlStrcmp(attr->name, (xmlChar *)Z_STRVAL_P(member)) && match_ns(sxe, (xmlNodePtr) attr, sxe->iter.nsprefix, sxe->iter.isprefix)) {
if ((!test || xmlStrEqual(attr->name, sxe->iter.name)) && xmlStrEqual(attr->name, (xmlChar *)Z_STRVAL_P(member)) && match_ns(sxe, (xmlNodePtr) attr, sxe->iter.nsprefix, sxe->iter.isprefix)) {
is_attr = 1;
++counter;
break;
Expand Down Expand Up @@ -558,7 +545,7 @@ static zval *sxe_prop_dim_write(zend_object *object, zval *member, zval *value,
while (node) {
SKIP_TEXT(node);

if (!xmlStrcmp(node->name, (xmlChar *)Z_STRVAL_P(member)) && match_ns(sxe, node, sxe->iter.nsprefix, sxe->iter.isprefix)) {
if (xmlStrEqual(node->name, (xmlChar *)Z_STRVAL_P(member)) && match_ns(sxe, node, sxe->iter.nsprefix, sxe->iter.isprefix)) {
newnode = node;
++counter;
}
Expand Down Expand Up @@ -647,7 +634,7 @@ static zval *sxe_property_get_adr(zend_object *object, zend_string *zname, int f
sxe = php_sxe_fetch_object(object);
GET_NODE(sxe, node);
name = ZSTR_VAL(zname);
node = sxe_get_element_by_name(sxe, node, &name, &type);
node = sxe_get_element_by_name(sxe, node, name, &type);
if (node) {
return NULL;
}
Expand Down Expand Up @@ -721,7 +708,7 @@ static int sxe_prop_dim_exists(zend_object *object, zval *member, int check_empt
int nodendx = 0;

while (attr && nodendx <= Z_LVAL_P(member)) {
if ((!test || !xmlStrcmp(attr->name, sxe->iter.name)) && match_ns(sxe, (xmlNodePtr) attr, sxe->iter.nsprefix, sxe->iter.isprefix)) {
if ((!test || xmlStrEqual(attr->name, sxe->iter.name)) && match_ns(sxe, (xmlNodePtr) attr, sxe->iter.nsprefix, sxe->iter.isprefix)) {
if (nodendx == Z_LVAL_P(member)) {
exists = 1;
break;
Expand All @@ -732,7 +719,7 @@ static int sxe_prop_dim_exists(zend_object *object, zval *member, int check_empt
}
} else {
while (attr) {
if ((!test || !xmlStrcmp(attr->name, sxe->iter.name)) && !xmlStrcmp(attr->name, (xmlChar *)Z_STRVAL_P(member)) && match_ns(sxe, (xmlNodePtr) attr, sxe->iter.nsprefix, sxe->iter.isprefix)) {
if ((!test || xmlStrEqual(attr->name, sxe->iter.name)) && xmlStrEqual(attr->name, (xmlChar *)Z_STRVAL_P(member)) && match_ns(sxe, (xmlNodePtr) attr, sxe->iter.nsprefix, sxe->iter.isprefix)) {
exists = 1;
break;
}
Expand All @@ -741,7 +728,7 @@ static int sxe_prop_dim_exists(zend_object *object, zval *member, int check_empt
}
}
if (exists && check_empty == 1 &&
(!attr->children || !attr->children->content || !attr->children->content[0] || !xmlStrcmp(attr->children->content, (const xmlChar *) "0")) ) {
(!attr->children || !attr->children->content || !attr->children->content[0] || xmlStrEqual(attr->children->content, (const xmlChar *) "0")) ) {
/* Attribute with no content in it's text node */
exists = 0;
}
Expand All @@ -760,7 +747,7 @@ static int sxe_prop_dim_exists(zend_object *object, zval *member, int check_empt
exists = 1;
if (check_empty == 1 &&
(!node->children || (node->children->type == XML_TEXT_NODE && !node->children->next &&
(!node->children->content || !node->children->content[0] || !xmlStrcmp(node->children->content, (const xmlChar *) "0")))) ) {
(!node->children->content || !node->children->content[0] || xmlStrEqual(node->children->content, (const xmlChar *) "0")))) ) {
exists = 0;
}
}
Expand Down Expand Up @@ -845,7 +832,7 @@ static void sxe_prop_dim_delete(zend_object *object, zval *member, bool elements
int nodendx = 0;

while (attr && nodendx <= Z_LVAL_P(member)) {
if ((!test || !xmlStrcmp(attr->name, sxe->iter.name)) && match_ns(sxe, (xmlNodePtr) attr, sxe->iter.nsprefix, sxe->iter.isprefix)) {
if ((!test || xmlStrEqual(attr->name, sxe->iter.name)) && match_ns(sxe, (xmlNodePtr) attr, sxe->iter.nsprefix, sxe->iter.isprefix)) {
if (nodendx == Z_LVAL_P(member)) {
xmlUnlinkNode((xmlNodePtr) attr);
php_libxml_node_free_resource((xmlNodePtr) attr);
Expand All @@ -858,7 +845,7 @@ static void sxe_prop_dim_delete(zend_object *object, zval *member, bool elements
} else {
while (attr) {
anext = attr->next;
if ((!test || !xmlStrcmp(attr->name, sxe->iter.name)) && !xmlStrcmp(attr->name, (xmlChar *)Z_STRVAL_P(member)) && match_ns(sxe, (xmlNodePtr) attr, sxe->iter.nsprefix, sxe->iter.isprefix)) {
if ((!test || xmlStrEqual(attr->name, sxe->iter.name)) && xmlStrEqual(attr->name, (xmlChar *)Z_STRVAL_P(member)) && match_ns(sxe, (xmlNodePtr) attr, sxe->iter.nsprefix, sxe->iter.isprefix)) {
xmlUnlinkNode((xmlNodePtr) attr);
php_libxml_node_free_resource((xmlNodePtr) attr);
break;
Expand All @@ -885,7 +872,7 @@ static void sxe_prop_dim_delete(zend_object *object, zval *member, bool elements

SKIP_TEXT(node);

if (!xmlStrcmp(node->name, (xmlChar *)Z_STRVAL_P(member)) && match_ns(sxe, node, sxe->iter.nsprefix, sxe->iter.isprefix)) {
if (xmlStrEqual(node->name, (xmlChar *)Z_STRVAL_P(member)) && match_ns(sxe, node, sxe->iter.nsprefix, sxe->iter.isprefix)) {
xmlUnlinkNode(node);
php_libxml_node_free_resource(node);
}
Expand Down Expand Up @@ -1010,7 +997,7 @@ static int sxe_prop_is_empty(zend_object *object) /* {{{ */
attr = node ? (xmlAttrPtr)node->properties : NULL;
test = sxe->iter.name && sxe->iter.type == SXE_ITER_ATTRLIST;
while (attr) {
if ((!test || !xmlStrcmp(attr->name, sxe->iter.name)) && match_ns(sxe, (xmlNodePtr)attr, sxe->iter.nsprefix, sxe->iter.isprefix)) {
if ((!test || xmlStrEqual(attr->name, sxe->iter.name)) && match_ns(sxe, (xmlNodePtr)attr, sxe->iter.nsprefix, sxe->iter.isprefix)) {
return 0;
}
attr = attr->next;
Expand Down Expand Up @@ -1120,7 +1107,7 @@ static HashTable *sxe_get_prop_hash(zend_object *object, int is_debug) /* {{{ */
ZVAL_UNDEF(&zattr);
test = sxe->iter.name && sxe->iter.type == SXE_ITER_ATTRLIST;
while (attr) {
if ((!test || !xmlStrcmp(attr->name, sxe->iter.name)) && match_ns(sxe, (xmlNodePtr)attr, sxe->iter.nsprefix, sxe->iter.isprefix)) {
if ((!test || xmlStrEqual(attr->name, sxe->iter.name)) && match_ns(sxe, (xmlNodePtr)attr, sxe->iter.nsprefix, sxe->iter.isprefix)) {
ZVAL_STR(&value, sxe_xmlNodeListGetString((xmlDocPtr) sxe->document->ptr, attr->children, 1));
namelen = xmlStrlen(attr->name);
if (Z_ISUNDEF(zattr)) {
Expand Down Expand Up @@ -2196,7 +2183,7 @@ static zend_function* php_sxe_find_fptr_count(zend_class_entry *ce)
int inherited = 0;

while (parent) {
if (parent == sxe_class_entry) {
if (parent == ce_SimpleXMLElement) {
break;
}
parent = parent->parent;
Expand Down Expand Up @@ -2254,7 +2241,7 @@ PHP_FUNCTION(simplexml_load_file)
char *ns = NULL;
size_t ns_len = 0;
zend_long options = 0;
zend_class_entry *ce= sxe_class_entry;
zend_class_entry *ce= ce_SimpleXMLElement;
zend_function *fptr_count;
bool isprefix = 0;

Expand All @@ -2274,7 +2261,7 @@ PHP_FUNCTION(simplexml_load_file)
}

if (!ce) {
ce = sxe_class_entry;
ce = ce_SimpleXMLElement;
fptr_count = NULL;
} else {
fptr_count = php_sxe_find_fptr_count(ce);
Expand All @@ -2299,7 +2286,7 @@ PHP_FUNCTION(simplexml_load_string)
char *ns = NULL;
size_t ns_len = 0;
zend_long options = 0;
zend_class_entry *ce= sxe_class_entry;
zend_class_entry *ce= ce_SimpleXMLElement;
zend_function *fptr_count;
bool isprefix = 0;

Expand Down Expand Up @@ -2327,7 +2314,7 @@ PHP_FUNCTION(simplexml_load_string)
}

if (!ce) {
ce = sxe_class_entry;
ce = ce_SimpleXMLElement;
fptr_count = NULL;
} else {
fptr_count = php_sxe_find_fptr_count(ce);
Expand Down Expand Up @@ -2405,7 +2392,7 @@ static xmlNodePtr php_sxe_iterator_fetch(php_sxe_object *sxe, xmlNodePtr node, i
if (sxe->iter.name) {
while (node) {
if (node->type == XML_ATTRIBUTE_NODE) {
if (!xmlStrcmp(node->name, sxe->iter.name) && match_ns(sxe, node, prefix, isprefix)) {
if (xmlStrEqual(node->name, sxe->iter.name) && match_ns(sxe, node, prefix, isprefix)) {
break;
}
}
Expand All @@ -2424,7 +2411,7 @@ static xmlNodePtr php_sxe_iterator_fetch(php_sxe_object *sxe, xmlNodePtr node, i
} else if (sxe->iter.type == SXE_ITER_ELEMENT && sxe->iter.name) {
while (node) {
if (node->type == XML_ELEMENT_NODE) {
if (!xmlStrcmp(node->name, sxe->iter.name) && match_ns(sxe, node, prefix, isprefix)) {
if (xmlStrEqual(node->name, sxe->iter.name) && match_ns(sxe, node, prefix, isprefix)) {
break;
}
}
Expand Down Expand Up @@ -2601,7 +2588,7 @@ PHP_FUNCTION(simplexml_import_dom)
zval *node;
php_libxml_node_object *object;
xmlNodePtr nodep = NULL;
zend_class_entry *ce = sxe_class_entry;
zend_class_entry *ce = ce_SimpleXMLElement;
zend_function *fptr_count;

if (zend_parse_parameters(ZEND_NUM_ARGS(), "o|C!", &node, &ce) == FAILURE) {
Expand All @@ -2626,7 +2613,7 @@ PHP_FUNCTION(simplexml_import_dom)

if (nodep && nodep->type == XML_ELEMENT_NODE) {
if (!ce) {
ce = sxe_class_entry;
ce = ce_SimpleXMLElement;
fptr_count = NULL;
} else {
fptr_count = php_sxe_find_fptr_count(ce);
Expand Down Expand Up @@ -2676,10 +2663,10 @@ ZEND_GET_MODULE(simplexml)
/* {{{ PHP_MINIT_FUNCTION(simplexml) */
PHP_MINIT_FUNCTION(simplexml)
{
sxe_class_entry = register_class_SimpleXMLElement(zend_ce_stringable, zend_ce_countable, spl_ce_RecursiveIterator);
sxe_class_entry->create_object = sxe_object_new;
sxe_class_entry->default_object_handlers = &sxe_object_handlers;
sxe_class_entry->get_iterator = php_sxe_get_iterator;
ce_SimpleXMLElement = register_class_SimpleXMLElement(zend_ce_stringable, zend_ce_countable, spl_ce_RecursiveIterator);
ce_SimpleXMLElement->create_object = sxe_object_new;
ce_SimpleXMLElement->default_object_handlers = &sxe_object_handlers;
ce_SimpleXMLElement->get_iterator = php_sxe_get_iterator;

memcpy(&sxe_object_handlers, &std_object_handlers, sizeof(zend_object_handlers));
sxe_object_handlers.offset = XtOffsetOf(php_sxe_object, zo);
Expand All @@ -2702,12 +2689,9 @@ PHP_MINIT_FUNCTION(simplexml)
sxe_object_handlers.get_closure = NULL;
sxe_object_handlers.get_gc = sxe_get_gc;

/* TODO: Why do we have two variables for this? */
ce_SimpleXMLElement = sxe_class_entry;

ce_SimpleXMLIterator = register_class_SimpleXMLIterator(ce_SimpleXMLElement);

php_libxml_register_export(sxe_class_entry, simplexml_export_node);
php_libxml_register_export(ce_SimpleXMLElement, simplexml_export_node);

return SUCCESS;
}
Expand All @@ -2716,7 +2700,7 @@ PHP_MINIT_FUNCTION(simplexml)
/* {{{ PHP_MSHUTDOWN_FUNCTION(simplexml) */
PHP_MSHUTDOWN_FUNCTION(simplexml)
{
sxe_class_entry = NULL;
ce_SimpleXMLElement = NULL;
return SUCCESS;
}
/* }}} */
Expand Down