Skip to content

Commit 196b459

Browse files
laanwjlaanwj
committed
Merge #23438: refactor: Use spans of std::byte in serialize
fa5d2e6 Remove unused char serialize (MarcoFalke) fa24493 Use spans of std::byte in serialize (MarcoFalke) fa65bbf span: Add BytePtr helper (MarcoFalke) Pull request description: This changes the serialize code (`.read()` and `.write()` functions) to take a `Span` instead of a pointer and size. This is a breaking change for the serialize interface, so at no additional cost we can also switch to `std::byte` (instead of using `char`). The benefits of using `Span`: * Less verbose and less fragile code when passing an already existing `Span`(-like) object to or from serialization The benefits of using `std::byte`: * `std::byte` can't accidentally be mistaken for an integer The goal here is to only change serialize to use spans of `std::byte`. If needed, `AsBytes`, `MakeUCharSpan`, ... can be used (temporarily) to pass spans of the right type. Other changes that are included here: * [#22167](#22167) (refactor: Remove char serialize by MarcoFalke) * [#21906](#21906) (Preserve const in cast on CTransactionSignatureSerializer by promag) ACKs for top commit: laanwj: Concept and code review ACK fa5d2e6 sipa: re-utACK fa5d2e6 Tree-SHA512: 08ee9eced5fb777cedae593b11e33660bed9a3e1711a7451a87b835089a96c99ce0632918bb4666a4e859c4d020f88fb50f2dd734216b0c3d1a9a704967ece6f
2 parents cf5bb04 + fa5d2e6 commit 196b459

29 files changed

+208
-199
lines changed

‎src/bench/checkblock.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@
1717
static void DeserializeBlockTest(benchmark::Bench& bench)
1818
{
1919
CDataStream stream(benchmark::data::block413567, SER_NETWORK, PROTOCOL_VERSION);
20-
char a = '\0';
21-
stream.write(&a, 1); // Prevent compaction
20+
std::byte a{0};
21+
stream.write({&a, 1}); // Prevent compaction
2222

2323
bench.unit("block").run([&] {
2424
CBlock block;
@@ -31,8 +31,8 @@ static void DeserializeBlockTest(benchmark::Bench& bench)
3131
static void DeserializeAndCheckBlockTest(benchmark::Bench& bench)
3232
{
3333
CDataStream stream(benchmark::data::block413567, SER_NETWORK, PROTOCOL_VERSION);
34-
char a = '\0';
35-
stream.write(&a, 1); // Prevent compaction
34+
std::byte a{0};
35+
stream.write({&a, 1}); // Prevent compaction
3636

3737
ArgsManager bench_args;
3838
const auto chainParams = CreateChainParams(bench_args, CBaseChainParams::MAIN);

‎src/bench/rpc_blockchain.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ struct TestBlockAndIndex {
2323
TestBlockAndIndex()
2424
{
2525
CDataStream stream(benchmark::data::block413567, SER_NETWORK, PROTOCOL_VERSION);
26-
char a = '\0';
27-
stream.write(&a, 1); // Prevent compaction
26+
std::byte a{0};
27+
stream.write({&a, 1}); // Prevent compaction
2828

2929
stream >> block;
3030

‎src/common/bloom.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ void CBloomFilter::insert(const COutPoint& outpoint)
6262
{
6363
CDataStream stream(SER_NETWORK, PROTOCOL_VERSION);
6464
stream << outpoint;
65-
insert(stream);
65+
insert(MakeUCharSpan(stream));
6666
}
6767

6868
bool CBloomFilter::contains(Span<const unsigned char> vKey) const
@@ -83,7 +83,7 @@ bool CBloomFilter::contains(const COutPoint& outpoint) const
8383
{
8484
CDataStream stream(SER_NETWORK, PROTOCOL_VERSION);
8585
stream << outpoint;
86-
return contains(stream);
86+
return contains(MakeUCharSpan(stream));
8787
}
8888

8989
bool CBloomFilter::IsWithinSizeConstraints() const

‎src/dbwrapper.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ class CDBIterator
147147
template<typename K> bool GetKey(K& key) {
148148
leveldb::Slice slKey = piter->key();
149149
try {
150-
CDataStream ssKey(MakeUCharSpan(slKey), SER_DISK, CLIENT_VERSION);
150+
CDataStream ssKey{MakeByteSpan(slKey), SER_DISK, CLIENT_VERSION};
151151
ssKey >> key;
152152
} catch (const std::exception&) {
153153
return false;
@@ -158,7 +158,7 @@ class CDBIterator
158158
template<typename V> bool GetValue(V& value) {
159159
leveldb::Slice slValue = piter->value();
160160
try {
161-
CDataStream ssValue(MakeUCharSpan(slValue), SER_DISK, CLIENT_VERSION);
161+
CDataStream ssValue{MakeByteSpan(slValue), SER_DISK, CLIENT_VERSION};
162162
ssValue.Xor(dbwrapper_private::GetObfuscateKey(parent));
163163
ssValue >> value;
164164
} catch (const std::exception&) {
@@ -244,7 +244,7 @@ class CDBWrapper
244244
dbwrapper_private::HandleError(status);
245245
}
246246
try {
247-
CDataStream ssValue(MakeUCharSpan(strValue), SER_DISK, CLIENT_VERSION);
247+
CDataStream ssValue{MakeByteSpan(strValue), SER_DISK, CLIENT_VERSION};
248248
ssValue.Xor(obfuscate_key);
249249
ssValue >> value;
250250
} catch (const std::exception&) {

‎src/hash.h‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -111,8 +111,9 @@ class CHashWriter
111111
int GetType() const { return nType; }
112112
int GetVersion() const { return nVersion; }
113113

114-
void write(const char *pch, size_t size) {
115-
ctx.Write((const unsigned char*)pch, size);
114+
void write(Span<const std::byte> src)
115+
{
116+
ctx.Write(UCharCast(src.data()), src.size());
116117
}
117118

118119
/** Compute the double-SHA256 hash of all data written to this object.
@@ -162,18 +163,18 @@ class CHashVerifier : public CHashWriter
162163
public:
163164
explicit CHashVerifier(Source* source_) : CHashWriter(source_->GetType(), source_->GetVersion()), source(source_) {}
164165

165-
void read(char* pch, size_t nSize)
166+
void read(Span<std::byte> dst)
166167
{
167-
source->read(pch, nSize);
168-
this->write(pch, nSize);
168+
source->read(dst);
169+
this->write(dst);
169170
}
170171

171172
void ignore(size_t nSize)
172173
{
173-
char data[1024];
174+
std::byte data[1024];
174175
while (nSize > 0) {
175176
size_t now = std::min<size_t>(nSize, 1024);
176-
read(data, now);
177+
read({data, now});
177178
nSize -= now;
178179
}
179180
}

‎src/net.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3091,11 +3091,11 @@ void CaptureMessage(const CAddress& addr, const std::string& msg_type, const Spa
30913091
CAutoFile f(fsbridge::fopen(path, "ab"), SER_DISK, CLIENT_VERSION);
30923092

30933093
ser_writedata64(f, now.count());
3094-
f.write(msg_type.data(), msg_type.length());
3094+
f.write(MakeByteSpan(msg_type));
30953095
for (auto i = msg_type.length(); i < CMessageHeader::COMMAND_SIZE; ++i) {
30963096
f << uint8_t{'\0'};
30973097
}
30983098
uint32_t size = data.size();
30993099
ser_writedata32(f, size);
3100-
f.write((const char*)data.data(), data.size());
3100+
f.write(AsBytes(data));
31013101
}

‎src/node/blockstorage.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -813,7 +813,7 @@ bool ReadRawBlockFromDisk(std::vector<uint8_t>& block, const FlatFilePos& pos, c
813813
}
814814

815815
block.resize(blk_size); // Zeroing of memory is intentional here
816-
filein.read((char*)block.data(), blk_size);
816+
filein.read(MakeWritableByteSpan(block));
817817
} catch (const std::exception& e) {
818818
return error("%s: Read from block file failed: %s for %s", __func__, e.what(), pos.ToString());
819819
}

‎src/psbt.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -399,7 +399,7 @@ bool DecodeBase64PSBT(PartiallySignedTransaction& psbt, const std::string& base6
399399

400400
bool DecodeRawPSBT(PartiallySignedTransaction& psbt, const std::string& tx_data, std::string& error)
401401
{
402-
CDataStream ss_data(MakeUCharSpan(tx_data), SER_NETWORK, PROTOCOL_VERSION);
402+
CDataStream ss_data(MakeByteSpan(tx_data), SER_NETWORK, PROTOCOL_VERSION);
403403
try {
404404
ss_data >> psbt;
405405
if (!ss_data.empty()) {

‎src/pubkey.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,14 +142,14 @@ class CPubKey
142142
{
143143
unsigned int len = size();
144144
::WriteCompactSize(s, len);
145-
s.write((char*)vch, len);
145+
s.write(AsBytes(Span{vch, len}));
146146
}
147147
template <typename Stream>
148148
void Unserialize(Stream& s)
149149
{
150150
const unsigned int len(::ReadCompactSize(s));
151151
if (len <= SIZE) {
152-
s.read((char*)vch, len);
152+
s.read(AsWritableBytes(Span{vch, len}));
153153
if (len != size()) {
154154
Invalidate();
155155
}

‎src/script/bitcoinconsensus.cpp‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,20 +22,23 @@ class TxInputStream
2222
m_remaining(txToLen)
2323
{}
2424

25-
void read(char* pch, size_t nSize)
25+
void read(Span<std::byte> dst)
2626
{
27-
if (nSize > m_remaining)
27+
if (dst.size() > m_remaining) {
2828
throw std::ios_base::failure(std::string(__func__) + ": end of data");
29+
}
2930

30-
if (pch == nullptr)
31+
if (dst.data() == nullptr) {
3132
throw std::ios_base::failure(std::string(__func__) + ": bad destination buffer");
33+
}
3234

33-
if (m_data == nullptr)
35+
if (m_data == nullptr) {
3436
throw std::ios_base::failure(std::string(__func__) + ": bad source buffer");
37+
}
3538

36-
memcpy(pch, m_data, nSize);
37-
m_remaining -= nSize;
38-
m_data += nSize;
39+
memcpy(dst.data(), m_data, dst.size());
40+
m_remaining -= dst.size();
41+
m_data += dst.size();
3942
}
4043

4144
template<typename T>

0 commit comments

Comments
 (0)