#36715 refactor: Enable bugprone-signed-char-misuse
| Reviewability | Reviews | Agreement | Size | ||||
|---|---|---|---|---|---|---|---|
| #36715 refactor: Enable bugprone-signed-char-misuse | Ready | 1 | Positive | 3 + 1 tests | |||
|
|
|
| ||||
Reviewability: ReadyStalePaused Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs. Size: lines added or modified outside tests, then in tests; greener = smaller. | |||||||
Goal
Enables the clang-tidy check bugprone-signed-char-misuse to detect dangerous sign extension and conversions of signed char types. Adjusts three call sites across base58 decoding, bit conversion utilities, and network tests using uniform initialization or explicit casts to satisfy the check.
Problem: Unintended sign extension of signed characters can lead to bugs such as out-of-bounds array reads or incorrect integer conversions, but clang-tidy does not currently flag these patterns in CI or developer builds.
Evidence of importance:
- PR description: 'This can help to prevent out-of-bounds reads such as the one removed in 43a33e513c4044dc01d92e77da76f2e98faaedb5, or prevent other bugs.'
Risk of a mistake: low (tooling, tests). The changes only enable a linter check and tweak initialization syntax without altering underlying logic.
Changed since the previous assessment: Now includes the complete patch diff; the contents and approach remain unchanged from the previous description.
Reviewability
Ready
Nothing known that would invalidate a review now.
Waiting on (from the thread read):
- nothing: no pending items or author responses awaited
Agreement
Positive
support without stated reasons, no open objection (jpk68, hebasto)
Thread notes: Added Concept ACK from hebasto; no objections or questions raised.
Objections, suggestions, and questions: none recorded.
Support:
- jpk68 (contributor) (ACK, 2026-10-30): (no reason given) [not substantive]
- hebasto (member) (Concept ACK, 2026-10-31): (no reason given) [not substantive]
Participants:
- jpk68 (contributor, support, 1 statements 2026-10-30 to 2026-10-30): ACK with commit hash
- hebasto (member, support, 1 statements 2026-10-31 to 2026-10-31): Concept ACK
Files
2 lines under test/bench/ci.
- src/base58.cpp +1/-1
- src/test/net_tests.cpp +1/-1
- src/util/strencodings.h +1/-1
- src/.clang-tidy +1/-0
Card
The short text the ranking pass compares across PRs.
Enables the clang-tidy check bugprone-signed-char-misuse to detect unintended sign extension and casts from signed characters, adjusting three existing code sites to satisfy the check. This helps developers prevent signed-char conversion bugs such as out-of-bounds array reads during static analysis. The author cites a historical out-of-bounds read bug fixed in commit 43a33e513c40 as motivation.
Data
thread read through 2026-10-31 (2 statements), head fa6efa5da5
labels: Refactoring