Use strmake over my_snprintf to copy strings
#3429
Open
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
3 tasks
ParadoxV5
commented
Aug 2, 2024
| @@ -66,7 +66,7 @@ int Xcurl(PGLOBAL g, PCSZ Http, PCSZ Uri, PCSZ filename) | |||
| my_snprintf(buf, sizeof(buf)-1, "%s/%s", Http, Uri); | |||
|
|
|||
| } else | |||
| my_snprintf(buf, sizeof(buf)-1, "%s", Http); | |||
| strmake(buf, Http, sizeof(buf)-2); | |||
There was a problem hiding this comment.
questionable double--1
There is no need to parse a format string if we just need to clone chars over. In fact, when MariaDB#3360 enabled `ATTRIBUTE_FORMAT` on `my_snprintf`, `-Werror=format-security` complained several of these that “format not a string literal and no format arguments”.
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.
MDEV-21978-WformatChecks onmy_vsnprintfand Users #3360 (comment) to make that PR more focused.strmakes take precedence.Description
What problem is the patch trying to solve?
There is no need to parse a format string if we just need to clone chars over (and truncate if the buffer overflows).
In fact, when #3360 enabled
ATTRIBUTE_FORMATonmy_snprintf,-Werror=format-securitycomplained several of these that “format not a string literal and no format arguments”.There are alternatives to
strmake, but I’ve determined on Zulip chat for #3360 thatstrmakeis the best fit.(Although, in one case,
strcpysuffices because it’s the first write to the buffer and will countably not overflow.)Do you think this patch might introduce side-effects in other parts of the server?
Unless I messed up – not at all? I just swapped function A with function B.
Release Notes
Um… General code cleanup?
How can this PR be tested?
This switch does not require new tests – existing tests will do.
Basing the PR against the correct MariaDB version
This is a bug fix, and the PR is based against the earliest maintained branch in which the bug can be reproduced.PR quality check