HomeClusterLabs Projects

Refactor: libcrmcommon: Consolidate both-non-NULL in mark_xml_changes()
df0a9e486af8Unpublished

Unpublished Commit ยท Learn More

Not On Permanent Ref: This commit is not an ancestor of any permanent ref.
This commit no longer exists in the repository. It may have been part of a branch which was deleted.This commit has been deleted in the repository: it is no longer reachable from any branch, tag, or ref.

Description

Refactor: libcrmcommon: Consolidate both-non-NULL in mark_xml_changes()

Move the old_child != NULL code in the new_child loop, up to the
old_child loop. This is another case where old_child and new_child are
both non-NULL; we already cover that case in the first loop.

There is a corner case here, where behavior changes but neither the old
behavior nor the new behavior seems correct. match_xml() doesn't
actually require an exact match. The signature is as follows, minus
types:

match_xml(haystack, needle)

  • If needle has an id attribute, then we look for a child of haystack with the same name (element type) that has an id attribute with the same value.
  • If needle does not have an id attribute, then we look for a child of haystack with the same name. We ignore the id attribute, if set.

This means the match is asymmetric. Suppose we have the following
old_xml:

<parent>

<child id="some_id"/>

</parent>

and the following new_xml:

<parent>

<child/>

</parent>

Further suppose that old_child and new_child refer to the child elements
in the respective trees above.

Then:

  • match_xml(new_xml, old_child) returns NULL, because old_child has an id and there is no child of new_xml with name "child" and the same id.
  • match_xml(old_xml, new_child) returns old_child, because new_child has no id, so it is free to match any child of old_xml named "child".

So we compute different changes depending on which way we check. I
believe it would work as follows:

  • match_xml(new_xml, old_child) will result in marking a deletion and a creation now. I believe previously it would have resulted in a move, which is inaccurate if the element did not change position.
  • match_xml(old_xml, new_child) will result in marking a change to the existing element (specifically, an attribute deletion).

This may or may not make a practical difference. Regression tests still
pass. I wouldn't be surprised if some ACL scenarios break though, for
example.

Note that things become even more complicated if there are multiple
children with the same name in old_xml, new_xml, or both. Especially if
some of them move around: match_xml() returns the FIRST matching child.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>

Details

Provenance
nrwahl2Authored on Mar 13 2025, 11:35 PM

Event Timeline

Commit No Longer Exists

This commit no longer exists in the repository.