Use functions guaranteed to be locale independent (IsDigit, ToLower) in {Format,Parse}Money(...), uint256::SetHex(...), etc. Remove the use of locale dependent boost::is_space(...) #14599

practicalswift · 2018-10-29T08:50:23Z

Use ToLower(...) instead of std::tolower. std::tolower is locale dependent.
Use IsDigit(...) instead of std::isdigit. Some implementations (e.g. Microsoft in 1252 codepage) may classify single-byte characters other than [0-9] as digits.
Update KNOWN_VIOLATIONS: Remove fixed violations.
~~Replace use of locale dependent Boost trim (boost::trim) with locale independent TrimString.~~
Use IsSpace(...) instead of boost::is_space

ken2812221 · 2018-10-29T09:08:29Z

~~utACK 25fc82ccf0d48b5c4d35d47b657c4ba5945c8d3c~~ require #include <utilstrencoding.h> in wallet disabled configure.

I wonder that if this linter has a Good job! ... locale dependency removed error.

practicalswift · 2018-10-29T10:19:03Z

@ken2812221 Thanks for the quick review! Please re-review :-)

The locale linter doesn't have the positive "Good job!" message (in the case a dependency in KNOWN_VIOLATIONS is removed). That would have been nice though :-)

src/uint256.cpp

practicalswift · 2018-10-30T16:00:28Z

Added commit:

Replace use of locale dependent Boost trim (boost::trim) with locale independent TrimString.

DrahtBot · 2018-10-30T16:35:01Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#13751 (Utils and libraries: Drops the boost/algorithm/string/split.hpp dependency by 251Labs)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

src/bitcoin-tx.cpp

laanwj · 2018-11-01T13:22:35Z

utACK apart from the rtrim to trim change

practicalswift · 2018-11-05T12:41:50Z

Rebased!

practicalswift · 2018-11-05T23:02:37Z

Added another commit: 2f3ae54ac6dd2fe36c2f469d3b96b2a7bfb5ce90 ("Use IsSpace(...) instead of boost::is_space")

Please review :-)

practicalswift · 2018-11-06T16:39:01Z

@MarcoFalke @laanwj I've now excluded the trim change to make reviewing easier. Please re-review :-)

maflcko · 2018-11-08T21:09:03Z

utACK 7c9f790

src/test/getarg_tests.cpp

hebasto · 2018-12-06T11:15:02Z

utACK 8931a95

maflcko · 2018-12-06T18:48:22Z

re-utACK 8931a95

practicalswift · 2018-12-28T21:03:59Z

@laanwj @ken2812221 Would you mind re-reviewing? :-)

laanwj · 2019-01-09T17:16:35Z

src/uint256.cpp


    // skip 0x
-    if (psz[0] == '0' && tolower(psz[1]) == 'x')
+    if (psz[0] == '0' && ToLower((unsigned char)psz[1]) == 'x')


It's kind of strange for a ToLower function to take an unsigned char (because std::strings index as char), but that's not introduced in this PR.

laanwj · 2019-01-09T17:17:46Z

re-utACK 8931a95

…git, ToLower) in {Format,Parse}Money(...), uint256::SetHex(...), etc. Remove the use of locale dependent boost::is_space(...) 8931a95 Include util/strencodings.h which is required for IsSpace(...) (practicalswift) 7c9f790 Update KNOWN_VIOLATIONS: Remove fixed violations (practicalswift) 587924f Use IsSpace(...) instead of boost::is_space (practicalswift) c5fd143 Use ToLower(...) instead of std::tolower (practicalswift) e70cc89 Use IsDigit(...) instead of std::isdigit (practicalswift) Pull request description: * Use `ToLower(...)` instead of `std::tolower`. `std::tolower` is locale dependent. * Use `IsDigit(...)` instead of `std::isdigit`. Some implementations (e.g. Microsoft in 1252 codepage) may classify single-byte characters other than `[0-9]` as digits. * Update `KNOWN_VIOLATIONS`: Remove fixed violations. * ~~Replace use of locale dependent Boost trim (`boost::trim`) with locale independent `TrimString`.~~ * Use` IsSpace(...)` instead of `boost::is_space` Tree-SHA512: defed016136b530b723fa185afdbd00410925a748856ba3afa4cee60f61a67617e30f304f2b9991a67b5fe075d9624f051e14342aee176f45fbc024d59e1aa82

Summary: Use IsDigit(...) instead of std::isdigit Use ToLower(...) instead of std::tolower Use IsSpace(...) instead of boost::is_space Update KNOWN_VIOLATIONS: Remove fixed violations --- Backport of Core [[bitcoin/bitcoin#14599 | PR14599]] Test Plan: ninja check Reviewers: #bitcoin_abc, jasonbcox, deadalnix Reviewed By: #bitcoin_abc, jasonbcox, deadalnix Subscribers: deadalnix, jasonbcox Differential Revision: https://reviews.bitcoinabc.org/D6294

…t (IsDigit, ToLower) in {Format,Parse}Money(...), uint256::SetHex(...), etc. Remove the use of locale dependent boost::is_space(...) 8931a95 Include util/strencodings.h which is required for IsSpace(...) (practicalswift) 7c9f790 Update KNOWN_VIOLATIONS: Remove fixed violations (practicalswift) 587924f Use IsSpace(...) instead of boost::is_space (practicalswift) c5fd143 Use ToLower(...) instead of std::tolower (practicalswift) e70cc89 Use IsDigit(...) instead of std::isdigit (practicalswift) Pull request description: * Use `ToLower(...)` instead of `std::tolower`. `std::tolower` is locale dependent. * Use `IsDigit(...)` instead of `std::isdigit`. Some implementations (e.g. Microsoft in 1252 codepage) may classify single-byte characters other than `[0-9]` as digits. * Update `KNOWN_VIOLATIONS`: Remove fixed violations. * ~~Replace use of locale dependent Boost trim (`boost::trim`) with locale independent `TrimString`.~~ * Use` IsSpace(...)` instead of `boost::is_space` Tree-SHA512: defed016136b530b723fa185afdbd00410925a748856ba3afa4cee60f61a67617e30f304f2b9991a67b5fe075d9624f051e14342aee176f45fbc024d59e1aa82

Merge bitcoin bitcoin#14585, bitcoin#14599: Use functions guaranteed to be locale independent

more info: https://wiki.qt.io/New_Signal_Slot_Syntax + https://stackoverflow.com/a/16795664 + Fix broken notificator on GNOME + Do not use systray on systems which do not support systray + Global specified name for CoinJoin to be constant through the build + Qt + SmartnodeTab at GUI should work with no wallet loaded + TxFee setting is non-static now [refs](bitcoin/bitcoin#12909) + Filter out macOS process serial number. [refs](bitcoin/bitcoin#17184) + Conversion to Windows args to be utf-8 string before parsing them to the Core/Wallet. [refs](bitcoin/bitcoin#13883) + Statically specified functions which are used widely along codebase to be Guaranteed Locale Independent. [refs](bitcoin/bitcoin#14599) + macOS bug-fixes [refs](bitcoin/bitcoin#17887) , [refs](bitcoin/bitcoin#15040) + refactored Timefunctions to be Thread-Safe + added support to multiwallet + refactored locks with fixes to deadlocks + boost::program_options and boost::chrono are NOT IN USE anymore and are deleted from the codebase + ArgsManager slightly refactored + Hidden Command-Line Arguments added + IsServiceEnabled (e.g. zmq, wallet) command_line arguments visibility activation (if certain service is not enabled, all cli commands related to that service will not be shown) + Other fixes and tweaks

…t (IsDigit, ToLower) in {Format,Parse}Money(...), uint256::SetHex(...), etc. Remove the use of locale dependent boost::is_space(...) 8931a95 Include util/strencodings.h which is required for IsSpace(...) (practicalswift) 7c9f790 Update KNOWN_VIOLATIONS: Remove fixed violations (practicalswift) 587924f Use IsSpace(...) instead of boost::is_space (practicalswift) c5fd143 Use ToLower(...) instead of std::tolower (practicalswift) e70cc89 Use IsDigit(...) instead of std::isdigit (practicalswift) Pull request description: * Use `ToLower(...)` instead of `std::tolower`. `std::tolower` is locale dependent. * Use `IsDigit(...)` instead of `std::isdigit`. Some implementations (e.g. Microsoft in 1252 codepage) may classify single-byte characters other than `[0-9]` as digits. * Update `KNOWN_VIOLATIONS`: Remove fixed violations. * ~~Replace use of locale dependent Boost trim (`boost::trim`) with locale independent `TrimString`.~~ * Use` IsSpace(...)` instead of `boost::is_space` Tree-SHA512: defed016136b530b723fa185afdbd00410925a748856ba3afa4cee60f61a67617e30f304f2b9991a67b5fe075d9624f051e14342aee176f45fbc024d59e1aa82

fanquake added the Refactoring label Oct 29, 2018

practicalswift force-pushed the no-more-locale branch 2 times, most recently from 389d144 to 336d6af Compare October 29, 2018 09:56

practicalswift force-pushed the no-more-locale branch from 336d6af to 124fdab Compare October 29, 2018 11:13

practicalswift commented Oct 29, 2018

View reviewed changes

src/uint256.cpp Outdated Show resolved Hide resolved

practicalswift force-pushed the no-more-locale branch 3 times, most recently from ee2433b to c49621b Compare October 30, 2018 15:59

DrahtBot mentioned this pull request Oct 30, 2018

Utils and libraries: Drops the boost/algorithm/string/split.hpp dependency #13751

Closed

laanwj reviewed Nov 1, 2018

View reviewed changes

src/bitcoin-tx.cpp Outdated Show resolved Hide resolved

DrahtBot added the Needs rebase label Nov 5, 2018

practicalswift force-pushed the no-more-locale branch from c49621b to b1d84cb Compare November 5, 2018 12:41

DrahtBot removed the Needs rebase label Nov 5, 2018

practicalswift force-pushed the no-more-locale branch 2 times, most recently from 02727e0 to 2f3ae54 Compare November 5, 2018 22:03

practicalswift added 4 commits November 6, 2018 17:32

Use IsDigit(...) instead of std::isdigit

e70cc89

Use ToLower(...) instead of std::tolower

c5fd143

Use IsSpace(...) instead of boost::is_space

587924f

Update KNOWN_VIOLATIONS: Remove fixed violations

7c9f790

practicalswift force-pushed the no-more-locale branch from 2f3ae54 to 7c9f790 Compare November 6, 2018 16:33

hebasto requested changes Dec 6, 2018

View reviewed changes

src/test/getarg_tests.cpp Show resolved Hide resolved

Include util/strencodings.h which is required for IsSpace(...)

8931a95

maflcko added this to the 0.18.0 milestone Dec 6, 2018

laanwj reviewed Jan 9, 2019

View reviewed changes

laanwj merged commit 8931a95 into bitcoin:master Jan 9, 2019

practicalswift deleted the no-more-locale branch April 10, 2021 19:36

barton2526 mentioned this pull request Aug 4, 2021

refactor: Use functions guaranteed to be locale independent (ToLower, IsDigit, IsSpace) gridcoin-community/Gridcoin-Research#2265

Merged

UdjinM6 added a commit to dashpay/dash that referenced this pull request Sep 9, 2021

Merge pull request #4403 from dzutte-cpp/backport_14585_14599

be2bd03

Merge bitcoin bitcoin#14585, bitcoin#14599: Use functions guaranteed to be locale independent

bitcoin locked as resolved and limited conversation to collaborators Aug 18, 2022

Use functions guaranteed to be locale independent (IsDigit, ToLower) in {Format,Parse}Money(...), uint256::SetHex(...), etc. Remove the use of locale dependent boost::is_space(...) #14599

Use functions guaranteed to be locale independent (IsDigit, ToLower) in {Format,Parse}Money(...), uint256::SetHex(...), etc. Remove the use of locale dependent boost::is_space(...) #14599

Uh oh!

Conversation

practicalswift commented Oct 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ken2812221 commented Oct 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

practicalswift commented Oct 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

practicalswift commented Oct 30, 2018

Uh oh!

DrahtBot commented Oct 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

Uh oh!

laanwj commented Nov 1, 2018

Uh oh!

practicalswift commented Nov 5, 2018

Uh oh!

practicalswift commented Nov 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

practicalswift commented Nov 6, 2018

Uh oh!

maflcko commented Nov 8, 2018

Uh oh!

Uh oh!

hebasto commented Dec 6, 2018

Uh oh!

maflcko commented Dec 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

practicalswift commented Dec 28, 2018

Uh oh!

laanwj Jan 9, 2019

Choose a reason for hiding this comment

Uh oh!

laanwj commented Jan 9, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

practicalswift commented Oct 29, 2018 •

edited

Loading

ken2812221 commented Oct 29, 2018 •

edited

Loading

practicalswift commented Oct 29, 2018 •

edited

Loading

DrahtBot commented Oct 30, 2018 •

edited

Loading

practicalswift commented Nov 5, 2018 •

edited

Loading

maflcko commented Dec 6, 2018 •

edited

Loading