Details

Reviewers

jasonbcox
deadalnix

Group Reviewers

Restricted Project

Commits

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

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

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

majcosta created this revision.May 29 2020, 17:30

Herald added a reviewer: Restricted Project. · View Herald TranscriptMay 29 2020, 17:30

majcosta requested review of this revision.May 29 2020, 17:30

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

Build Bitcoin ABC Diffs / Diff Testing (build-werror) started.

Build Bitcoin ABC Diffs / Diff Testing (build-diff) started.

Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) started.

Build Bitcoin ABC Diffs / Diff Testing (build-werror) passed.

Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) passed.

Build Bitcoin ABC Diffs / Diff Testing (build-diff) passed.

Harbormaster completed remote builds in B11085: Diff 20706.May 29 2020, 17:40

majcosta planned changes to this revision.May 29 2020, 17:49

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

Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) started.

Build Bitcoin ABC Diffs / Diff Testing (build-diff) started.

Build Bitcoin ABC Diffs / Diff Testing (build-werror) started.

majcosta added a child revision: D6295: [backport#15139] util: Make ToLower and ToUpper take a char.May 29 2020, 18:04

Build Bitcoin ABC Diffs / Diff Testing (build-werror) passed.

Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) passed.

Build Bitcoin ABC Diffs / Diff Testing (build-diff) passed.

Harbormaster completed remote builds in B11086: Diff 20707.May 29 2020, 18:13

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

In D6294#155360, @jasonbcox wrote:

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.

replaced C-style cast

Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) started.

Build Bitcoin ABC Diffs / Diff Testing (build-diff) started.

Build Bitcoin ABC Diffs / Diff Testing (build-werror) started.

Build Bitcoin ABC Diffs / Diff Testing (build-werror) passed.

Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) passed.

Build Bitcoin ABC Diffs / Diff Testing (build-diff) passed.

Harbormaster completed remote builds in B11103: Diff 20731.May 30 2020, 02:45

deadalnix requested changes to this revision.May 31 2020, 22:01

deadalnix added inline comments.

src/uint256.cpp
33 ↗	(On Diff #20731)	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.