Replace some strstr() calls with exact matching against gchar * arrays #3999
+751
−555
Conversation
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
Contributor
Author
|
Added four commits on top. About to rebase on main (which will make one of them go away). |
Signed-off-by: Reid Wahl <[email protected]>
* Return bool instead of gboolean. * Take a (const char *) instead of (const gchar *) argument. * Use more descriptive variable names. * Drop redundant "\note" lines from doxygen. Signed-off-by: Reid Wahl <[email protected]>
This is optional and adds a bit of code, but it uses the abstraction that GLib provides for this purpose. It's fine if we decide not to keep this. Signed-off-by: Reid Wahl <[email protected]>
We have several places where we set a return code, and then we invert it. This makes it a little bit easier to think about. Also check whether attr_name is NULL before printing it. Signed-off-by: Reid Wahl <[email protected]>
To make it easier to convert to pcmk__g_strv_contains(). Signed-off-by: Reid Wahl <[email protected]>
Nothing uses this yet. The implementation is copied from g_strv_contains(). Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
The existing strstr() call matched too broadly. For example, given PCMK_debug="asdfpacemaker-controld1234" we would determine that debugging was enabled for pacemaker-controld. Instead, treat the value as a comma-delimited list, as documented. Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
The existing strstr() calls matched too broadly. For example, given PCMK_valgrind_enabled="asdfpacemaker-controld1234" we would determine that valgrind was enabled for pacemaker-controld. Instead, treat the value as a comma-delimited list, as documented. Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
It's a substring of op->stderr_data, which we keep without truncating. If we keep a larger string, I don't see a reason not to keep a smaller string. Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
g_strchomp() is simpler and clearer. Signed-off-by: Reid Wahl <[email protected]>
The node_types (now types) argument is expected to be a comma-separated list, even though this is inconsistently enforced at the moment. Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
We only cared about whether it was 0. Signed-off-by: Reid Wahl <[email protected]>
Previously, we matched too broadly using strstr(). For example, if types="asdfcluster1234", then we would output cluster nodes. We matched somewhat more strictly for remote nodes, but still would have matched types="remote1234". Now, split on commas and match exactly. Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
In my opinion the passes variable, which gets inverted at the end, made things more confusing. Also remove some nesting and use const where possible. Signed-off-by: Reid Wahl <[email protected]>
And check for "CRM_meta_" instead of just "CRM_meta". Shouldn't matter in practice but seems more correct. Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
This feature was introduced by efbede9 and is undocumented. Currently the only thing that uses it is the CIB remote client, which sets it to PCMK__XA_PASSWORD. So we could actually do a direct string match. But for extensibility, I'm treating the value as a comma-separated list of hidden attributes, so that a client can pass multiple attribute names to be hidden. It may be harder to add this feature if we do it later, because older servers won't recognize multi-name values from newer clients. In any case, using strstr() matched too broadly. Also, go ahead and call pcmk__xml_escape() even on the constant "*****" that we use for hidden values. It simplifies the code. Signed-off-by: Reid Wahl <[email protected]>
As far as I can tell, the clients that set crm_system_name to a value that contains "sbd" set it to a value that STARTS with "sbd". Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
All call paths guarantee non-NULL action. It doesn't really matter whether we return true or false here. Returning true preserves existing behavior. Signed-off-by: Reid Wahl <[email protected]>
Even though it adds more code, it makes more sense than building a string of discrete actions delimited by spaces. We can join the list when we need a string. We also avoid strstr(), which could give a false positive if one action is a substring of another. Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
We escape newlines as "\\n" (or "\\\\n" in C) only if pcmk__xml_escape_attr_pretty is set. In the past, when unescape_newlines() was added via c820651, we escaped all newlines that way in XML attributes. It's not immediately clear what would have caused newlines in action stdout and stderr to get escaped back then, but the important thing is that nothing appears to do it now. Signed-off-by: Reid Wahl <[email protected]>
Also note that we now case the GQuark to long long. GQuark is documented as a non-zero integer, even though it's implemented as a guint32. Signed-off-by: Reid Wahl <[email protected]>
Previously, we matched too broadly using strstr(). For example, if trace_files="asdfcluster.c1234", then we would trace file cluster.c. Now, split on commas and match exactly. Note that we need to store the resulting arrays in file-scope static variables so that we can free them later. Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
...params for digests. Signed-off-by: Reid Wahl <[email protected]>
Of course this adds overhead -- now we're allocating enough memory to hold a copy of the output argument to crm_log_output_fn() that's split up by line. It certainly improves readability, however. Signed-off-by: Reid Wahl <[email protected]>
Replace with an identical call to do_crm_log() Signed-off-by: Reid Wahl <[email protected]>
Reduce duplication. Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
nrwahl2
force-pushed
the
nrwahl2-strstr
branch
from
e0c540a to
10dc82b
Compare
This seems clearer than fiddling with strchr(). Signed-off-by: Reid Wahl <[email protected]>
Coverity just caught this for some reason. Signed-off-by: Reid Wahl <[email protected]>
nrwahl2
force-pushed
the
nrwahl2-strstr
branch
from
10dc82b to
7e81f6b
Compare
Contributor
Author
|
Coverity found a memory leak that's been present since 97827b9. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.