diff --git a/include/crm/common/xml_element_internal.h b/include/crm/common/xml_element_internal.h index ec2964c2504..4878cc0ffaa 100644 --- a/include/crm/common/xml_element_internal.h +++ b/include/crm/common/xml_element_internal.h @@ -37,6 +37,12 @@ extern "C" { const char *pcmk__xe_add_last_written(xmlNode *xe); +bool pcmk__xe_foreach_attr(xmlNode *xml, bool (*fn)(xmlAttr *, void *), + void *user_data); +bool pcmk__xe_foreach_const_attr(const xmlNode *xml, + bool (*fn)(const xmlAttr *, void *), + void *user_data); + xmlNode *pcmk__xe_first_child(const xmlNode *parent, const char *node_name, const char *attr_n, const char *attr_v); diff --git a/include/crm/pengine/internal.h b/include/crm/pengine/internal.h index 543d1a92023..cb31eaca444 100644 --- a/include/crm/pengine/internal.h +++ b/include/crm/pengine/internal.h @@ -246,7 +246,7 @@ pe_base_name_eq(const pcmk_resource_t *rsc, const char *id) // Number of characters in rsc->id before any clone suffix size_t base_len = pe_base_name_end(rsc->id) - rsc->id + 1; - return (strlen(id) == base_len) && !strncmp(id, rsc->id, base_len); + return (strlen(id) == base_len) && g_str_has_prefix(rsc->id, id); } return false; } diff --git a/lib/common/acl.c b/lib/common/acl.c index ce34b93d6cc..0349dc10d50 100644 --- a/lib/common/acl.c +++ b/lib/common/acl.c @@ -764,30 +764,28 @@ is_mode_allowed(uint32_t flags, enum pcmk__xml_flags mode) return false; } - if (pcmk__is_set(flags, mode)) { - // The access we requested is explicitly allowed - return true; - } - - if ((mode == pcmk__xf_acl_read) - && pcmk__is_set(flags, pcmk__xf_acl_write)) { + switch (mode) { + case pcmk__xf_acl_read: + // Write access provides read access + return pcmk__any_flags_set(flags, + pcmk__xf_acl_read|pcmk__xf_acl_write); - // Write access provides read access - return true; - } + case pcmk__xf_acl_write: + return pcmk__is_set(flags, pcmk__xf_acl_write); - if ((mode == pcmk__xf_acl_create) - && pcmk__any_flags_set(flags, pcmk__xf_acl_write|pcmk__xf_created)) { + case pcmk__xf_acl_create: + /* Write access provides create access. + * + * @TODO Why does the \c pcmk__xf_created flag provide create + * access? This was introduced by commit e2ed85fe. + */ + return pcmk__any_flags_set(flags, + pcmk__xf_acl_write|pcmk__xf_created); - /* Write access provides create access. - * - * @TODO Why does the \c pcmk__xf_created flag provide create access? - * This was introduced by commit e2ed85fe. - */ - return true; + default: + // Invalid mode + return false; } - - return false; } /*! @@ -951,28 +949,29 @@ xml_acl_filtered_copy(const char *user, xmlNode *acl_source, xmlNode *xml, * * \param[in] xml XML element to check * - * \return true if XML element is implicitly allowed, false otherwise + * \return \c true if XML element is implicitly allowed, or \c false otherwise */ static bool -implicitly_allowed(const xmlNode *xml) +implicitly_allowed(xmlNode *xml) { - GString *path = NULL; + for (xmlAttr *attr = pcmk__xe_first_attr(xml); attr != NULL; + attr = attr->next) { - for (xmlAttr *prop = xml->properties; prop != NULL; prop = prop->next) { - if (strcmp((const char *) prop->name, PCMK_XA_ID) != 0) { + if (attr_is_not_id(attr, NULL)) { return false; } } - path = pcmk__element_xpath(xml); - pcmk__assert(path != NULL); - - if (strstr((const char *) path->str, "/" PCMK_XE_ACLS "/") != NULL) { - g_string_free(path, TRUE); - return false; + /* Creation is not implicitly allowed for a descendant of PCMK_XE_ACLS, but + * it may be for PCMK_XE_ACLS itself. Start checking at xml->parent and walk + * up the tree. + */ + for (xml = xml->parent; xml != NULL; xml = xml->parent) { + if (pcmk__xe_is(xml, PCMK_XE_ACLS)) { + return false; + } } - g_string_free(path, TRUE); return true; } diff --git a/lib/common/actions.c b/lib/common/actions.c index b312c2ac612..26a7d2c8b1e 100644 --- a/lib/common/actions.c +++ b/lib/common/actions.c @@ -266,7 +266,8 @@ match_before(const char *key, size_t position, const char **matches) const size_t possible = position - match_len - 1; if ((key[possible] == '_') - && (strncmp(key + possible + 1, matches[i], match_len) == 0)) { + && g_str_has_prefix(key + possible + 1, matches[i])) { + return possible; } } diff --git a/lib/common/digest.c b/lib/common/digest.c index 3552ba5ccce..80448e62f54 100644 --- a/lib/common/digest.c +++ b/lib/common/digest.c @@ -300,10 +300,10 @@ pcmk__xa_filterable(const char *name) static bool should_filter_for_digest(xmlAttrPtr a, void *user_data) { - if (strncmp((const char *) a->name, CRM_META "_", - sizeof(CRM_META " ") - 1) == 0) { + if (g_str_has_prefix((const char *) a->name, CRM_META "_")) { return true; } + return pcmk__str_any_of((const char *) a->name, PCMK_XA_ID, PCMK_XA_CRM_FEATURE_SET, diff --git a/lib/common/ipc_client.c b/lib/common/ipc_client.c index fd8a6058290..4f782e9b8a4 100644 --- a/lib/common/ipc_client.c +++ b/lib/common/ipc_client.c @@ -1747,8 +1747,9 @@ pcmk__ipc_is_authentic_process_active(const char *name, uid_t refuid, } rc = pcmk_rc_ok; - if ((found_uid != refuid || found_gid != refgid) - && strncmp(last_asked_name, name, sizeof(last_asked_name))) { + if (((found_uid != refuid) || (found_gid != refgid)) + && !pcmk__str_eq(name, last_asked_name, pcmk__str_none)) { + if ((found_uid == 0) && (refuid != 0)) { pcmk__warn("Daemon (IPC %s) runs as root, whereas the expected " "credentials are %lld:%lld, hazard of violating the " diff --git a/lib/common/xml.c b/lib/common/xml.c index 5b34ba2fe5a..8ad4e190ec8 100644 --- a/lib/common/xml.c +++ b/lib/common/xml.c @@ -245,23 +245,75 @@ free_deleted_object(void *data) } } -// Free and NULL user, ACLs, and deleted objects in an XML node's private data +/*! + * \internal + * \brief Allocate and initialize private data for an XML document + * + * \param[in,out] doc XML document + */ static void -reset_xml_private_data(xml_doc_private_t *docpriv) +new_doc_private_data(xmlDoc *doc) { - if (docpriv != NULL) { - pcmk__assert(docpriv->check == PCMK__XML_DOC_PRIVATE_MAGIC); + xml_doc_private_t *priv = pcmk__assert_alloc(1, sizeof(xml_doc_private_t)); - g_clear_pointer(&docpriv->acl_user, free); - g_clear_pointer(&docpriv->acls, pcmk__free_acls); + priv->check = PCMK__XML_DOC_PRIVATE_MAGIC; + doc->_private = priv; +} - if(docpriv->deleted_objs) { - g_list_free_full(docpriv->deleted_objs, free_deleted_object); - docpriv->deleted_objs = NULL; - } +/*! + * \internal + * \brief Allocate and initialize private data for a non-document XML node + * + * \param[in,out] xml XML node + */ +static void +new_node_private_data(xmlNode *xml) +{ + const bool tracking = pcmk__xml_doc_all_flags_set(xml->doc, + pcmk__xf_tracking); + xml_node_private_t *priv = pcmk__assert_alloc(1, + sizeof(xml_node_private_t)); + + priv->check = PCMK__XML_NODE_PRIVATE_MAGIC; + xml->_private = priv; + + if (tracking) { + pcmk__set_xml_flags(priv, pcmk__xf_created); + pcmk__mark_xml_node_dirty(xml); } } +/*! + * \internal + * \brief Allocate and initialize private data for an XML attribute + * + * \param[in,out] attr XML attribute + * \param[in] user_data Ignored + * + * \return \c true (to continue iterating) + * + * \note This is compatible with \c pcmk__xe_foreach_attr(). + */ +static bool +new_attr_private_data(xmlAttr *attr, void *user_data) +{ + new_node_private_data((xmlNode *) attr); + return true; +} + +/*! + * \internal + * \brief Allocate and initialize private data for an XML element + * + * \param[in,out] xml XML element + */ +static void +new_element_private_data(xmlNode *xml) +{ + new_node_private_data(xml); + pcmk__xe_foreach_attr(xml, new_attr_private_data, NULL); +} + /*! * \internal * \brief Allocate and initialize private data for an XML node @@ -276,47 +328,25 @@ reset_xml_private_data(xml_doc_private_t *docpriv) static bool new_private_data(xmlNode *node, void *user_data) { - bool tracking = false; - CRM_CHECK(node != NULL, return true); if (node->_private != NULL) { return true; } - tracking = pcmk__xml_doc_all_flags_set(node->doc, pcmk__xf_tracking); - switch (node->type) { case XML_DOCUMENT_NODE: - { - xml_doc_private_t *docpriv = - pcmk__assert_alloc(1, sizeof(xml_doc_private_t)); - - docpriv->check = PCMK__XML_DOC_PRIVATE_MAGIC; - node->_private = docpriv; - } - break; + new_doc_private_data((xmlDoc *) node); + return true; - case XML_ELEMENT_NODE: case XML_ATTRIBUTE_NODE: case XML_COMMENT_NODE: - { - xml_node_private_t *nodepriv = - pcmk__assert_alloc(1, sizeof(xml_node_private_t)); - - nodepriv->check = PCMK__XML_NODE_PRIVATE_MAGIC; - node->_private = nodepriv; - if (tracking) { - pcmk__set_xml_flags(nodepriv, pcmk__xf_dirty|pcmk__xf_created); - } - - for (xmlAttr *iter = pcmk__xe_first_attr(node); iter != NULL; - iter = iter->next) { + new_node_private_data(node); + return true; - new_private_data((xmlNode *) iter, user_data); - } - } - break; + case XML_ELEMENT_NODE: + new_element_private_data(node); + return true; case XML_TEXT_NODE: case XML_DTD_NODE: @@ -327,13 +357,95 @@ new_private_data(xmlNode *node, void *user_data) CRM_LOG_ASSERT(node->type == XML_ELEMENT_NODE); return true; } +} - if (tracking) { - pcmk__mark_xml_node_dirty(node); +/*! + * \internal + * \brief Free and zero all data fields of an XML document's private data + * + * This function does not clear the \c check field or free the private data + * object itself. + * + * \param[in,out] docpriv XML document private data + */ +static void +reset_doc_private_data(xml_doc_private_t *docpriv) +{ + if (docpriv == NULL) { + return; } + + pcmk__assert(docpriv->check == PCMK__XML_DOC_PRIVATE_MAGIC); + + docpriv->flags = pcmk__xf_none; + + g_clear_pointer(&docpriv->acl_user, free); + g_clear_pointer(&docpriv->acls, pcmk__free_acls); + + g_list_free_full(docpriv->deleted_objs, free_deleted_object); + docpriv->deleted_objs = NULL; +} + +/*! + * \internal + * \brief Free and clear private data for an XML document + * + * \param[in,out] doc XML document + */ +static void +free_doc_private_data(xmlDoc *doc) +{ + reset_doc_private_data(doc->_private); + g_clear_pointer(&doc->_private, free); +} + +/*! + * \internal + * \brief Free and clear private data for a non-document XML node + * + * \param[in,out] xml XML node + */ +static void +free_node_private_data(xmlNode *xml) +{ + xml_node_private_t *nodepriv = xml->_private; + + pcmk__assert(nodepriv->check == PCMK__XML_NODE_PRIVATE_MAGIC); + + g_clear_pointer(&xml->_private, free); +} + +/*! + * \internal + * \brief Free and clear private data for an XML attribute + * + * \param[in,out] attr XML attribute + * \param[in] user_data Ignored + * + * \return \c true (to continue iterating) + * + * \note This is compatible with \c pcmk__xe_foreach_attr(). + */ +static bool +free_attr_private_data(xmlAttr *xml, void *user_data) +{ + free_node_private_data((xmlNode *) xml); return true; } +/*! + * \internal + * \brief Free and clear private data for an XML element and its attributes + * + * \param[in,out] xml XML element + */ +static void +free_element_private_data(xmlNode *xml) +{ + free_node_private_data(xml); + pcmk__xe_foreach_attr(xml, free_attr_private_data, NULL); +} + /*! * \internal * \brief Free private data for an XML node @@ -354,23 +466,19 @@ free_private_data(xmlNode *node, void *user_data) return true; } - if (node->type == XML_DOCUMENT_NODE) { - reset_xml_private_data((xml_doc_private_t *) node->_private); - - } else { - xml_node_private_t *nodepriv = node->_private; - - pcmk__assert(nodepriv->check == PCMK__XML_NODE_PRIVATE_MAGIC); + switch (node->type) { + case XML_DOCUMENT_NODE: + free_doc_private_data((xmlDoc *) node); + return true; - for (xmlAttr *iter = pcmk__xe_first_attr(node); iter != NULL; - iter = iter->next) { + case XML_ELEMENT_NODE: + free_element_private_data(node); + return true; - free_private_data((xmlNode *) iter, user_data); - } + default: + free_node_private_data(node); + return true; } - free(node->_private); - node->_private = NULL; - return true; } /*! @@ -478,8 +586,7 @@ pcmk__xml_commit_changes(xmlDoc *doc) pcmk__xml_tree_foreach(xmlDocGetRootElement(doc), commit_attr_deletions, NULL); } - reset_xml_private_data(docpriv); - docpriv->flags = pcmk__xf_none; + reset_doc_private_data(docpriv); } /*! @@ -1083,6 +1190,26 @@ pcmk__xml_escape(const char *text, enum pcmk__xml_escape_type type) return g_string_free(copy, FALSE); } +/*! + * \internal + * \brief Set the \c pcmk__xf_created flag on an attribute + * + * \param[in,out] attr XML attribute + * \param[in] user_data Ignored + * + * \return \c true (to continue iterating) + * + * \note This is compatible with \c pcmk__xe_foreach_attr(). + */ +static bool +mark_attr_created(xmlAttr *attr, void *user_data) +{ + xml_node_private_t *nodepriv = attr->_private; + + pcmk__set_xml_flags(nodepriv, pcmk__xf_created); + return true; +} + /*! * \internal * \brief Add an XML attribute to a node, marked as deleted @@ -1093,12 +1220,11 @@ pcmk__xml_escape(const char *text, enum pcmk__xml_escape_type type) * differences have been calculated. * * \param[in,out] new_xml XML to modify - * \param[in] element Name of XML element that changed (for logging) * \param[in] attr_name Name of attribute that was deleted * \param[in] old_value Value of attribute that was deleted */ static void -mark_attr_deleted(xmlNode *new_xml, const char *element, const char *attr_name, +mark_attr_deleted(xmlNode *new_xml, const char *attr_name, const char *old_value) { xml_doc_private_t *docpriv = new_xml->doc->_private; @@ -1121,7 +1247,7 @@ mark_attr_deleted(xmlNode *new_xml, const char *element, const char *attr_name, pcmk__xa_remove(attr, false); pcmk__trace("XML attribute %s=%s was removed from %s", attr_name, old_value, - element); + (const char *) new_xml->name); } /* @@ -1129,14 +1255,14 @@ mark_attr_deleted(xmlNode *new_xml, const char *element, const char *attr_name, * \brief Check ACLs for a changed XML attribute */ static void -mark_attr_changed(xmlNode *new_xml, const char *element, const char *attr_name, +mark_attr_changed(xmlNode *new_xml, const char *attr_name, const char *old_value) { xml_doc_private_t *docpriv = new_xml->doc->_private; char *vcopy = pcmk__xe_get_copy(new_xml, attr_name); pcmk__trace("XML attribute %s was changed from '%s' to '%s' in %s", - attr_name, old_value, vcopy, element); + attr_name, old_value, vcopy, (const char *) new_xml->name); // Restore the original value (without checking ACLs) pcmk__clear_xml_flags(docpriv, pcmk__xf_tracking); @@ -1153,20 +1279,19 @@ mark_attr_changed(xmlNode *new_xml, const char *element, const char *attr_name, * \brief Mark an XML attribute as having changed position * * \param[in,out] new_xml XML to modify - * \param[in] element Name of XML element that changed (for logging) * \param[in,out] old_attr Attribute that moved, in original XML * \param[in,out] new_attr Attribute that moved, in \p new_xml * \param[in] p_old Ordinal position of \p old_attr in original XML * \param[in] p_new Ordinal position of \p new_attr in \p new_xml */ static void -mark_attr_moved(xmlNode *new_xml, const char *element, xmlAttr *old_attr, - xmlAttr *new_attr, int p_old, int p_new) +mark_attr_moved(xmlNode *new_xml, xmlAttr *old_attr, xmlAttr *new_attr, + int p_old, int p_new) { xml_node_private_t *nodepriv = new_attr->_private; pcmk__trace("XML attribute %s moved from position %d to %d in %s", - old_attr->name, p_old, p_new, element); + old_attr->name, p_old, p_new, (const char *) new_xml->name); // Mark document, element, and all element's parents as changed pcmk__mark_xml_node_dirty(new_xml); @@ -1180,93 +1305,112 @@ mark_attr_moved(xmlNode *new_xml, const char *element, xmlAttr *old_attr, /*! * \internal - * \brief Calculate differences in all previously existing XML attributes + * \brief Mark an XML attribute as deleted, changed, or moved if appropriate * - * \param[in,out] old_xml Original XML to compare - * \param[in,out] new_xml New XML to compare + * Given an attribute (from an old XML element) and a new XML element, check + * whether the attribute has been deleted, changed, or moved between the old and + * new elements. If so, mark the new XML element to indicate what changed. + * + * \param[in,out] old_attr XML attribute from old element + * \param[in,out] user_data New XML element + * + * \return \c true (to continue iterating) + * + * \note This is compatible with \c pcmk__xe_foreach_attr(). */ -static void -xml_diff_old_attrs(xmlNode *old_xml, xmlNode *new_xml) -{ - xmlAttr *attr_iter = pcmk__xe_first_attr(old_xml); - - while (attr_iter != NULL) { - const char *name = (const char *) attr_iter->name; - xmlAttr *old_attr = attr_iter; - xmlAttr *new_attr = xmlHasProp(new_xml, attr_iter->name); - const char *old_value = pcmk__xml_attr_value(attr_iter); - - attr_iter = attr_iter->next; - if (new_attr == NULL) { - mark_attr_deleted(new_xml, (const char *) old_xml->name, name, - old_value); - - } else { - xml_node_private_t *nodepriv = new_attr->_private; - int new_pos = pcmk__xml_position((xmlNode*) new_attr, - pcmk__xf_skip); - int old_pos = pcmk__xml_position((xmlNode*) old_attr, - pcmk__xf_skip); - const char *new_value = pcmk__xe_get(new_xml, name); - - // This attribute isn't new - pcmk__clear_xml_flags(nodepriv, pcmk__xf_created); - - if (strcmp(new_value, old_value) != 0) { - mark_attr_changed(new_xml, (const char *) old_xml->name, name, - old_value); - - } else if ((old_pos != new_pos) - && !pcmk__xml_doc_all_flags_set(new_xml->doc, - pcmk__xf_ignore_attr_pos - |pcmk__xf_tracking)) { - /* pcmk__xf_tracking is always set by pcmk__xml_mark_changes() - * before this function is called, so only the - * pcmk__xf_ignore_attr_pos check is truly relevant. - */ - mark_attr_moved(new_xml, (const char *) old_xml->name, - old_attr, new_attr, old_pos, new_pos); - } - } +static bool +mark_attr_diff(xmlAttr *old_attr, void *user_data) +{ + xmlNode *new_xml = user_data; + + const char *name = (const char *) old_attr->name; + + xmlAttr *new_attr = xmlHasProp(new_xml, old_attr->name); + xml_node_private_t *new_priv = NULL; + + const char *old_value = pcmk__xml_attr_value(old_attr); + const char *new_value = NULL; + + int old_pos = 0; + int new_pos = 0; + + if (new_attr == NULL) { + mark_attr_deleted(new_xml, name, old_value); + return true; + } + + new_priv = new_attr->_private; + new_value = pcmk__xe_get(new_xml, name); + + // This attribute isn't new + pcmk__clear_xml_flags(new_priv, pcmk__xf_created); + + if (!pcmk__str_eq(old_value, new_value, pcmk__str_none)) { + mark_attr_changed(new_xml, name, old_value); + return true; + } + + old_pos = pcmk__xml_position((xmlNode *) old_attr, pcmk__xf_skip); + new_pos = pcmk__xml_position((xmlNode *) new_attr, pcmk__xf_skip); + + if ((old_pos == new_pos) + || pcmk__xml_doc_all_flags_set(new_xml->doc, + pcmk__xf_ignore_attr_pos)) { + return true; } + + mark_attr_moved(new_xml, old_attr, new_attr, old_pos, new_pos); + return true; } /*! * \internal - * \brief Check all attributes in new XML for creation + * \brief Mark a new attribute dirty if ACLs allow creation, or remove otherwise + * + * We set the \c pcmk__xf_created flag on all attributes in the new XML at an + * earlier stage of change calculation. Then we checked whether each attribute + * was present in the old XML, and we cleared the flag if so. If the flag is + * still set, then the attribute is truly new. + * + * Now we check whether ACLs allow the attribute's creation. If so, we "accept" + * it: we mark the attribute as dirty and modified, and we mark all of its + * parents as dirty. Otherwise, we reject it by removing the attribute (ignoring + * ACLs and change tracking for the removal). + * + * \param[in,out] attr XML attribute to mark dirty or remove + * \param[in] user_data Ignored * - * For each of a given XML element's attributes marked as newly created, accept - * (and mark as dirty) or reject the creation according to ACLs. + * \return \c true (to continue iterating) * - * \param[in,out] new_xml XML to check + * \note This is compatible with \c pcmk__xe_foreach_attr(). */ -static void -mark_created_attrs(xmlNode *new_xml) +static bool +check_new_attr_acls(xmlAttr *attr, void *user_data) { - xmlAttr *attr_iter = pcmk__xe_first_attr(new_xml); - - while (attr_iter != NULL) { - xmlAttr *new_attr = attr_iter; - xml_node_private_t *nodepriv = attr_iter->_private; + const char *name = (const char *) attr->name; + const char *value = pcmk__xml_attr_value(attr); + const xml_node_private_t *nodepriv = attr->_private; + xmlNode *new_xml = attr->parent; + const char *new_xml_id = pcmk__s(pcmk__xe_id(new_xml), "without ID"); - attr_iter = attr_iter->next; - if (pcmk__is_set(nodepriv->flags, pcmk__xf_created)) { - const char *attr_name = (const char *) new_attr->name; - - pcmk__trace("Created new attribute %s=%s in %s", attr_name, - pcmk__xml_attr_value(new_attr), new_xml->name); + if (!pcmk__is_set(nodepriv->flags, pcmk__xf_created)) { + return true; + } - /* Check ACLs (we can't use the remove-then-create trick because it - * would modify the attribute position). - */ - if (pcmk__check_acl(new_xml, attr_name, pcmk__xf_acl_write)) { - pcmk__mark_xml_attr_dirty(new_attr); - } else { - // Creation was not allowed, so remove the attribute - pcmk__xa_remove(new_attr, true); - } - } + /* Check ACLs (we can't use the remove-then-create trick because it + * would modify the attribute position). + */ + if (!pcmk__check_acl(new_xml, name, pcmk__xf_acl_write)) { + pcmk__trace("ACLs prevent creation of attribute %s=%s in %s %s", name, + value, (const char *) new_xml->name, new_xml_id); + pcmk__xa_remove(attr, true); + return true; } + + pcmk__trace("Created new attribute %s=%s in %s %s", name, value, + (const char *) new_xml->name, new_xml_id); + pcmk__mark_xml_attr_dirty(attr); + return true; } /*! @@ -1280,15 +1424,10 @@ static void xml_diff_attrs(xmlNode *old_xml, xmlNode *new_xml) { // Cleared later if attributes are not really new - for (xmlAttr *attr = pcmk__xe_first_attr(new_xml); attr != NULL; - attr = attr->next) { - xml_node_private_t *nodepriv = attr->_private; - - pcmk__set_xml_flags(nodepriv, pcmk__xf_created); - } + pcmk__xe_foreach_attr(new_xml, mark_attr_created, NULL); - xml_diff_old_attrs(old_xml, new_xml); - mark_created_attrs(new_xml); + pcmk__xe_foreach_attr(old_xml, mark_attr_diff, new_xml); + pcmk__xe_foreach_attr(new_xml, check_new_attr_acls, NULL); } /*! diff --git a/lib/common/xml_attr.c b/lib/common/xml_attr.c index c7993bbab2f..4ccfbdeb91a 100644 --- a/lib/common/xml_attr.c +++ b/lib/common/xml_attr.c @@ -50,26 +50,30 @@ pcmk__xa_remove(xmlAttr *attr, bool force) return pcmk_rc_ok; } + if (force) { + goto remove; + } + element = attr->parent; - if (!force && !pcmk__check_acl(element, NULL, pcmk__xf_acl_write)) { + if (!pcmk__check_acl(element, NULL, pcmk__xf_acl_write)) { // ACLs apply to element, not to particular attributes pcmk__trace("ACLs prevent removal of attributes from %s element", element->name); return EPERM; } - if (!force && (element != NULL) - && pcmk__xml_doc_all_flags_set(element->doc, pcmk__xf_tracking)) { - + if (pcmk__xml_doc_all_flags_set(element->doc, pcmk__xf_tracking)) { // Leave in place (marked for removal) until after diff is calculated pcmk__xml_set_parent_flags(element, pcmk__xf_dirty); pcmk__set_xml_flags((xml_node_private_t *) attr->_private, pcmk__xf_deleted); - } else { - pcmk__xml_free_private_data((xmlNode *) attr); - xmlRemoveProp(attr); + return pcmk_rc_ok; } + +remove: + pcmk__xml_free_private_data((xmlNode *) attr); + xmlRemoveProp(attr); return pcmk_rc_ok; } diff --git a/lib/common/xml_element.c b/lib/common/xml_element.c index 78a2a084963..4bafab83425 100644 --- a/lib/common/xml_element.c +++ b/lib/common/xml_element.c @@ -26,6 +26,67 @@ #include #include "crmcommon_private.h" +/*! + * \internal + * \brief Call a function for each of an XML element's attributes + * + * \param[in,out] xml XML element + * \param[in] fn Function to call for each attribute (returns + * \c true to continue iterating over attributes or + * \c false to stop) + * \param[in,out] user_data User data argument for \p fn + * + * \return \c false if any \p fn call returned \c false, or \c true otherwise + * + * \note \p fn may remove its attribute argument. + */ +bool +pcmk__xe_foreach_attr(xmlNode *xml, bool (*fn)(xmlAttr *, void *), + void *user_data) +{ + xmlAttr *attr = pcmk__xe_first_attr(xml); + + while (attr != NULL) { + xmlAttr *next = attr->next; + + if (!fn(attr, user_data)) { + return false; + } + + attr = next; + } + + return true; +} + +/*! + * \internal + * \brief Call a function for each of a \c const XML element's attributes + * + * \param[in] xml XML element + * \param[in] fn Function to call for each attribute (returns + * \c true to continue iterating over attributes or + * \c false to stop) + * \param[in,out] user_data User data argument for \p fn + * + * \return \c false if any \p fn call returned \c false, or \c true otherwise + */ +bool +pcmk__xe_foreach_const_attr(const xmlNode *xml, + bool (*fn)(const xmlAttr *, void *), + void *user_data) +{ + for (const xmlAttr *attr = pcmk__xe_first_attr(xml); attr != NULL; + attr = attr->next) { + + if (!fn(attr, user_data)) { + return false; + } + } + + return true; +} + /*! * \internal * \brief Find first XML child element matching given criteria diff --git a/lib/common/xpath.c b/lib/common/xpath.c index 8bff702dcb0..f3ff8bf2ac0 100644 --- a/lib/common/xpath.c +++ b/lib/common/xpath.c @@ -272,10 +272,11 @@ pcmk__xpath_find_one(xmlDoc *doc, const char *path, uint8_t level) * * \param[in] xml The XML element for which to build an XPath string * - * \return A \p GString that matches \p xml, or \p NULL if \p xml is \p NULL. + * \return \c GString that matches \p xml, or \c NULL if \p xml is \c NULL + * (guaranteed not to be \c NULL if \p xml is not \c NULL) * * \note The caller is responsible for freeing the string using - * \p g_string_free(). + * \c g_string_free(). */ GString * pcmk__element_xpath(const xmlNode *xml)