Refactor: libcrmcommon: Use XML attribute foreach functions in more places#4124
Open
nrwahl2 wants to merge 33 commits into
Open
Refactor: libcrmcommon: Use XML attribute foreach functions in more places#4124nrwahl2 wants to merge 33 commits into
nrwahl2 wants to merge 33 commits into
Conversation
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>
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 file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Next batch from #4013