Page MenuHomePhabricator

refactor: Treat CDataStream bytes as uint8_t
ClosedPublic

Authored by PiRK on Oct 26 2023, 13:23.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABC5e862abc23cc: refactor: Treat CDataStream bytes as uint8_t
Summary

Using uint8_t for raw bytes has a style benefit:

  • The signedness is clear from reading the code, as it does not depend on the architecture

Other clean-ups in this pull include:

  • Remove unused methods
  • Constructor is simplified with Span
  • Remove Init() member in favor of C++11 member initialization

Notes:

  • the += operatior is not unused in our codebase
  • the seeder code needs minor adjustments to use the Span constructor

This is a backport of core#20464

Test Plan

ninja all check-all bench-bitcoin bitcoin-fuzzers

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

PiRK requested review of this revision.Oct 26 2023, 13:23
Fabien requested changes to this revision.Oct 26 2023, 14:25
Fabien added a subscriber: Fabien.
Fabien added inline comments.
src/test/streams_tests.cpp
218 ↗(On Diff #42814)
This revision now requires changes to proceed.Oct 26 2023, 14:25
PiRK requested review of this revision.Oct 26 2023, 15:09
PiRK added inline comments.
src/test/streams_tests.cpp
218 ↗(On Diff #42814)

This one does not compile.

PiRK planned changes to this revision.Oct 26 2023, 15:12

Trying to figure out when core removed void insert(iterator it, const char *first, const char *last)

In D14689#329993, @PiRK wrote:

Trying to figure out when core removed void insert(iterator it, const char *first, const char *last)

They still had it when this PR was merged. It is just the streams_empty_vector test that is specific to bitcoin ABC.

PiRK requested review of this revision.Oct 26 2023, 15:18
This revision is now accepted and ready to land.Oct 26 2023, 15:30