-
Notifications
You must be signed in to change notification settings - Fork 336
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.
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
XML refactors: crm_xml_init() through crm_copy_xml_element() #3450
Closed
Conversation
This file contains 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
nrwahl2
force-pushed
the
nrwahl2-xml_attrs2
branch
9 times, most recently
from
May 6, 2024 12:48
56551bb
to
072875c
Compare
nrwahl2
changed the title
XML deprecations: crm_xml_init() through crm_xml_set_id()
XML refactors: crm_xml_init() through crm_copy_xml_element()
May 7, 2024
To replace crm_xml_init() Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
To replace crm_xml_cleanup() Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
It's only two lines and not Pacemaker-specific; this is just the standard way to free an XML node that may be part of a document. Behavior is preserved in all three callers. However, for posterity: * For free_xml_with_position(), we've already done any other checks that are necessary. * For pcmk__strip_xml_text(), a text node can't be the document root, so we don't have to worry about freeing the document. ACLs and change tracking aren't relevant for the current callers; stripping text is done only internally and is essentially an implementation detail. * For pcmk__apply_creation_acl(), we don't want to recheck ACLs (which would fail). Change tracking is irrelevant because the newly created node hasn't been accepted to the CIB yet. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
To replace free_xml() Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
To replace expand_idref() 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>
Call teardown function to free global XML data structures Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Previously, if attr->set_id were the empty string, attrd_set_id() would pass a copy to crm_xml_sanitize_id(). In that case, there's nothing to sanitize and thus no way for crm_xml_sanitize_id() to turn its argument into a valid XML ID (other than performing a realloc() and storing arbitrary charactrs). It's better to treat an empty attr->set_id the same as a NULL one, and use the default set ID in that case. (It is possible for attr->set_id to be the empty string because it may be arbitrary user input from CLI tools.) As of this commit, all callers of crm_xml_sanitize_id() are guaranteed to pass a non-empty string as the argument. This is technically a behavioral change, but it's quite an edge case, and the existing behavior is problematic. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Not used yet Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
This runs surprisingly fast considering that there are over 1 million test cases. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Not used yet Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
This runs surprisingly fast considering that there are over 1 million test cases. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Preserve the body of xml_patch_versions() because the new function returns EINVAL in the edge case where we fail to parse version numbers as integers. The order of arguments changes. I think it's clearer to use "source" and then "target". In v1 patchsets, we used "add"/"remove", but we can start moving away from that. 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>
Preserve body of patchset_process_digest() because the new function returns without adding a digest if the target is dirty. Drop with_digest argument, and instead check in the callers. There are only two callers, so it's not terribly clunky. It felt awkward to take an argument for "should we add a digest?" in a function whose purpose is to add a digest. 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>
And limit line length of SUMMARY Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
This is highly unfortunate, but changing it would break backward compatibility. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
It's simpler to use multiple strings than to have a boolean represent whether a file name should be treated as a raw string. When we can make the input precedence sane at a compatibility break, an enum for input source will likely make the most sense. Then we can bring back callbacks to set the enum value.. Also rename "apply" to "patch" for clarity, and use G_OPTION_FLAG_NONE. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
It was for logging only. It's inaccurate if we're using a string or stdin as our input source, and it's not particularly helpful. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
...in crm_diff.c. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
It doesn't copy an element, so I'd like to deprecate it. It's useful, but only slightly, and we only call it four times. If we decide to re-add this functionality internally, we could consider adding a NULL-terminated array argument to pcmk__xe_copy_attrs(). If NULL, copy all attrs. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Not much to do here. Mainly doxygen, dropping a redundant call to pcmk__xml_track_changes(), and an extra comment. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Improve doxygen and variable names. Clarify/simplify logic. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
We don't have to operate on a copy of input. We're not using it for anything else after calling this function. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
I fail to see how this is useful. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
pcmk__log_xml_patchset() already logs these, almost identically, at notice level. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Not really necessary but easy and this is the last piece Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
nrwahl2
force-pushed
the
nrwahl2-xml_attrs2
branch
from
May 7, 2024 21:58
072875c
to
84ce4a3
Compare
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.
No description provided.