Skip to content

Refactor: libcrmcommon: Use XML attribute foreach functions in more places#4124

Open
nrwahl2 wants to merge 33 commits into
ClusterLabs:mainfrom
nrwahl2:nrwahl2-acls_first
Open

Refactor: libcrmcommon: Use XML attribute foreach functions in more places#4124
nrwahl2 wants to merge 33 commits into
ClusterLabs:mainfrom
nrwahl2:nrwahl2-acls_first

Conversation

@nrwahl2
Copy link
Copy Markdown
Contributor

@nrwahl2 nrwahl2 commented Jun 4, 2026

Next batch from #4013

nrwahl2 added 30 commits June 4, 2026 11:23
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
There is only one caller besides the recursive call. patchset is
guaranteed to be non-NULL.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
pcmk__element_xpath() is guaranteed to return non-NULL unless it's
argument is NULL.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
This change is larger than I'd prefer, but everything here is tied
together and I think review will be easier if we do it in one step.

In particular, I thought that the previous "create the change element
if it's NULL" on every iteration if we found a changed attribute, made
the whole thing harder to read. So at the cost of a bit of extra memory
allocation (for a GSList) and iteration, we move to the following
approach:

1. Make a list of changed or deleted attributes. If the resulting list
   is empty, stop. Otherwise, continue to next steps.
2. Create a PCMK_XE_CHANGE child of the patchset.
3. Create a PCMK_XE_CHANGE_LIST child of the PCMK_XE_CHANGE.
4. For each changed or deleted attribute, add a PCMK_XE_CHANGE_ATTR
   child to the PCMK_XE_CHANGE_LIST.
5. Create a PCMK_XE_CHANGE_RESULT child of the PCMK_XE_CHANGE.
6. Create an xml->name child of the PCMK_XE_CHANGE_RESULT.
7. For each attribute that was not deleted, copy it from the source xml
   to the xml->name child.
8. Free the list of changed attributes.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
This doesn't change behavior. It's probably a very slight optimization,
but I'm doing this mostly to align with the pcmk__xml_flags guards on
the other "add_X_change()" calls.

Also rename the function to add_changes_to_patchset() so that it's
shorter.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
If show_xml_element() wanted to show an attribute with a NULL value, it
would use "<null>" for the value. This didn't make sense. Immediately
prior, we XML-escaped the value. "<null>" has XML special characters and
would need to be escaped in order to be valid. (Of course,
pcmk__xml_show() and its helpers are currently used only with text-like
output formats; for those, having perfectly valid XML is less
important.)

We could use a different fallback string, such as "(null)". However, for
any XML element that we create, every attribute in its properties list
should have a non-NULL value. So this should have little or no practical
effect. "<null>" was a fallback in case of bugs, strange inputs via the
public API (for example, do_crm_log_xml()), or other unexpected
scenarios. As far as I know, we never promised that NULL attributes
would be represented in the output. So I prefer to simply not show them.

This will soon allow us to call pcmk__xml_dump_attr() in
show_xml_element().

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
They do the same thing for attributes, except that show_xml_element()
hides hidden attributes. If pcmk__dump_xml_attr() ever hides hidden
attributes, then we can reduce duplication even further. For now, I'm
not sure what side effects might occur if we hide attributs in
pcmk__dump_xml_attr().

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
An attribute's value really shouldn't be NULL unless the XML came from
public API (and even then, it would be strange). pcmk__dump_xml_attr()
already ignores attributes with NULL values. So dropping the NULL check
simply means that if an attribute's name is in PCMK__XA_HIDDEN and its
value is NULL, we'll now show "*****" for its value instead of skipping
it. That should basically never happen in practice, and if it does, this
behavior is arguably more correct anyway.

Also, an attribute's name can't be empty in valid XML.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
From data to xml. I want to use the name data for something else in an
upcoming commit, and we typically use "xml" for xmlNode * arguments.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
nrwahl2 added 3 commits June 4, 2026 11:35
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
@nrwahl2 nrwahl2 requested a review from clumens June 4, 2026 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant