#36715 refactor: Enable bugprone-signed-char-misuse

ReviewabilityReviewsAgreementSize
#36715 maflcko refactor: Enable bugprone-signed-char-misuseReady1Positive3 + 1 tests
  • Catch dangerous signed character sign extensions and conversions automatically in developer builds and CI
  • Prevent bugs such as out-of-bounds array reads caused by unexpected signed char conversions
  • Ready to review
  • Approval without stated reasons (jpk68)
  • Concept approval without stated reasons (hebasto)
  • No objections or concerns raised
  • 3 lines added or modified outside tests
  • 1 lines added or modified in tests
  • 3 lines removed in total
  • 4 files, 1 commits
Uncategorized15 PRs, unranked
2 rejected
  • P2P: not a member. This PR enables a clang-tidy linter check across the codebase with minor mechanical adjustments; it does not target P2P protocol, connection management, or transport logic.
  • Utilities (logging, arguments, libraries): not a member. Touches src/util/strencodings.h only to satisfy a newly enabled clang-tidy linter check in src/.clang-tidy.
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

dossier JSON · extract JSON · model ledger, generated 2026-11-01T01:17, confidence high, input hash d39d2681ef5c299e
thread read through 2026-10-31 (2 statements), head fa6efa5da5
labels: Refactoring