-
Notifications
You must be signed in to change notification settings - Fork 38.7k
refactor: Treat CDataStream bytes as uint8_t #20464
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
fa75da2 to
fa565eb
Compare
|
Concept ACK: nice cleanup (+49 −109)! |
Also, rename CSerializeData to SerializeData
fa565eb to
fa5a769
Compare
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
|
Concept ACK! |
|
There are some more unnecessary (possibly more but I could find these quickly) |
|
Concept ACK |
|
Code review ACK apart from the above comment. This PR is straightforward to review. The first commit can be seen separately. |
|
Concept ACK -- will review as soon as #20464 (comment) is resolved |
fa8dec9 to
fa29272
Compare
theStack
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK fa29272 🍾
|
code review ACK fa29272 |
fa29272 Remove redundant MakeUCharSpan wrappers (MarcoFalke) faf4aa2 Remove CDataStream::Init in favor of C++11 member initialization (MarcoFalke) fada14b Treat CDataStream bytes as uint8_t (MarcoFalke) fa8bdb0 refactor: Drop CDataStream constructors in favor of one taking a Span of bytes (MarcoFalke) faa96f8 Remove unused CDataStream methods (MarcoFalke) Pull request description: 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 ACKs for top commit: laanwj: code review ACK fa29272 theStack: ACK fa29272 🍾 Tree-SHA512: 931ee28bd99843d7e894b48e90e1187ffb0278677c267044b3c0c255069d9bbd9298ab2e539b1002a30b543d240450eaec718ef4ee95a7fd4be0a295e926343f
Follow the updates from PR bitcoin#20464. Assume the new SerializeData (former CSerializeData) is now a stream of uint8_t and drop the usage of the GetAndClear() function, which was removed in the referred PR.
Follow the updates from PR bitcoin#20464. Assume the new SerializeData (former CSerializeData) is now a stream of uint8_t and drop the usage of the GetAndClear() function, which was removed in the referred PR.
Using
uint8_tfor raw bytes has a style benefit:Other clean-ups in this pull include:
SpanInit()member in favor of C++11 member initialization