Page MenuHomePhabricator

streams: Create VectorReader stream interface for vectors.
ClosedPublic

Authored by markblundeberg on Apr 12 2019, 02:00.

Details

Summary

This is a read analogue for the existing CVectorWriter.

For BIP158; is cherry-picked from two commits (947133d and 87f2d9e) from:
https://github.com/bitcoin/bitcoin/pull/12254/commits

In addition, the buggy seek() method was removed per later backport:
https://github.com/bitcoin/bitcoin/pull/14357

Test Plan

make check

Diff Detail

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

Event Timeline

markblundeberg edited the test plan for this revision. (Show Details)
markblundeberg edited the summary of this revision. (Show Details)
markblundeberg edited the test plan for this revision. (Show Details)

squashed in child commit

Fabien requested changes to this revision.Apr 12 2019, 09:39
Fabien added a subscriber: Fabien.
Fabien added inline comments.
src/streams.h
124 ↗(On Diff #8040)
/**
 * Minimal stream for reading from an existing vector by reference
 */
138 ↗(On Diff #8040)

This comment block contains doxygen tags, it should be a doxygen block.

148 ↗(On Diff #8040)

Dito (even if the (other params same as above) part will make no sense in a doxygen html output).

183 ↗(On Diff #8040)

I find it weird that the position gets changed to something out of bound even if the exception is thrown.
The behavior of the read() function seems more logical to me. Thought ?

src/test/streams_tests.cpp
99 ↗(On Diff #8040)

Move comment above.

110 ↗(On Diff #8040)

Dito.

This revision now requires changes to proceed.Apr 12 2019, 09:39
deadalnix requested changes to this revision.Apr 12 2019, 11:11
deadalnix added inline comments.
src/test/streams_tests.cpp
76 ↗(On Diff #8040)

Use types from cstdint.

@Fabien that would make for a good linter that can auto-correct.

markblundeberg added inline comments.
src/streams.h
183 ↗(On Diff #8040)

I agree; looks like this seek() method got abandoned in a later commit anyway. (I'm gonna have to be more careful with the git blame forward-looking :-) ) https://github.com/bitcoin/bitcoin/commit/958e1a307e92f6678459ebdcd64e05b61f5f3a77#diff-eb3b977a68473a9d20093cbe90c659e6

markblundeberg edited the summary of this revision. (Show Details)
  • remove seek method per later backport
  • adjust comments style per comments
  • use cstdint types
This revision is now accepted and ready to land.Apr 16 2019, 07:20