test: Remove last uses of snprintf and simplify #27036
Merged
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
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. |
One test case uses snprintf to convert an int to a string. Change it to use ToString (which uses a locale-independent version of std::to_string). Also remove unnecessary parts of StringContentsSerializer.
c91783b to
b803229
Compare
|
Concept ACK. Maybe verify if the linter files need updating (test/lint/lint-locale-dependence.py?) |
Thanks! Pushed that change about a half hour ago. You were too quick 😄 |
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Feb 6, 2023
b803229 Remove use of snprintf and simplify (John Moffett) Pull request description: These are the only remaining uses of `snprintf` in our project, and they can cause unexpected issues -- for example, see bitcoin#27014. Change them to use our `ToString` (which uses a locale-independent version of `std::to_string`) to convert an `int` to `std::string`. Also remove resulting unused parts of `StringContentsSerializer`. Closes bitcoin#27014 ACKs for top commit: Sjors: tACK b803229, fixes bitcoin#27014. Tree-SHA512: c903977e654711929decafe8887d0de13b38a340d7082875acc5d41950d834dcfde074e9cabecaf5f9a760f62c34322297b4b156af29761650ef5803b1a54b59
fanquake
added a commit
that referenced
this pull request
Feb 17, 2023
30a3230 script: remove out-of-date snprintf TODO (Jon Atack) 0e01514 net: remove orphaned CSubNet::SanityCheck() (Jon Atack) Pull request description: `CSubNet::SanityCheck()` was added in #20140, and not removed in #22570 when it became orphaned code. Also, remove an out-of-date `snprintf` TODO that was resolved in #27036, and fix up 2 words to make the spelling linter green again. ACKs for top commit: fanquake: ACK 30a3230 pinheadmz: ACK 30a3230 brunoerg: crACK 30a3230 Tree-SHA512: f91a2a5af902d3b82ab496f19deeac17d58dbf72a8016e880ea61ad858b66e7ea0ae70b964c4032018eb3252cc34ac5fea163131c6a7f1baf87fc9ec9b5833d8
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Feb 17, 2023
30a3230 script: remove out-of-date snprintf TODO (Jon Atack) 0e01514 net: remove orphaned CSubNet::SanityCheck() (Jon Atack) Pull request description: `CSubNet::SanityCheck()` was added in bitcoin#20140, and not removed in bitcoin#22570 when it became orphaned code. Also, remove an out-of-date `snprintf` TODO that was resolved in bitcoin#27036, and fix up 2 words to make the spelling linter green again. ACKs for top commit: fanquake: ACK 30a3230 pinheadmz: ACK 30a3230 brunoerg: crACK 30a3230 Tree-SHA512: f91a2a5af902d3b82ab496f19deeac17d58dbf72a8016e880ea61ad858b66e7ea0ae70b964c4032018eb3252cc34ac5fea163131c6a7f1baf87fc9ec9b5833d8
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
These are the only remaining uses of
snprintfin our project, and they can cause unexpected issues -- for example, see #27014. Change them to use ourToString(which uses a locale-independent version ofstd::to_string) to convert aninttostd::string. Also remove resulting unused parts ofStringContentsSerializer.Closes #27014