Page MenuHomePhabricator

[backport#14599] Use functions guaranteed to be locale independent
ClosedPublic

Authored by majcosta on May 29 2020, 17:30.

Details

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 PR14599

Test Plan
ninja check

Event Timeline

[Bot Message]
One or more PR numbers were detected in the summary.
Links to those PRs have been inserted into the summary for reference.

reverted some core changes to locale-dependence linter that were wrong

jasonbcox requested changes to this revision.May 29 2020, 20:26
jasonbcox added a subscriber: jasonbcox.

Are the missed changes in LocaleDependenceLinter due to missing backports? If so, why not do those first?

src/uint256.cpp
33 ↗(On Diff #20707)

cast should be function style: uint8_t(...)

This revision now requires changes to proceed.May 29 2020, 20:26
deadalnix requested changes to this revision.May 29 2020, 21:16
deadalnix added a subscriber: deadalnix.
deadalnix added inline comments.
src/uint256.cpp
33 ↗(On Diff #20706)

C++ constructor instead of C cast

Are the missed changes in LocaleDependenceLinter due to missing backports? If so, why not do those first?

I've looked for them, but didn't find them. Looks like a legit mistake or they reintroduced them later. They're calling strtoll and strtoull in util/strencodings.cpp today.

deadalnix requested changes to this revision.May 31 2020, 22:01
deadalnix added inline comments.
src/uint256.cpp
33

You did change the behavior here, and it's fishy tests are passing. You might want to use that opportunity to add a handful of test cases.

This revision now requires changes to proceed.May 31 2020, 22:01

fixed error that changed behavior in uint256.cpp which should now be caught by D6323

This revision is now accepted and ready to land.Jun 2 2020, 18:06