refactor: Drop CDataStream constructors in favor of one taking a Span of bytes

MarcoFalke · MarcoFalke · commit fa8bdb048e65 · 2020-11-23T21:19:22.000+01:00
diff --git a/src/dbwrapper.h b/src/dbwrapper.h
@@ -8,9 +8,10 @@
 #include <clientversion.h>
 #include <fs.h>
 #include <serialize.h>
+#include <span.h>
 #include <streams.h>
-#include <util/system.h>
 #include <util/strencodings.h>
+#include <util/system.h>
 
 #include <leveldb/db.h>
 #include <leveldb/write_batch.h>
@@ -146,7 +147,7 @@ class CDBIterator
     template<typename K> bool GetKey(K& key) {
         leveldb::Slice slKey = piter->key();
         try {
-            CDataStream ssKey(slKey.data(), slKey.data() + slKey.size(), SER_DISK, CLIENT_VERSION);
+            CDataStream ssKey(MakeUCharSpan(slKey), SER_DISK, CLIENT_VERSION);
             ssKey >> key;
         } catch (const std::exception&) {
             return false;
@@ -157,7 +158,7 @@ class CDBIterator
     template<typename V> bool GetValue(V& value) {
         leveldb::Slice slValue = piter->value();
         try {
-            CDataStream ssValue(slValue.data(), slValue.data() + slValue.size(), SER_DISK, CLIENT_VERSION);
+            CDataStream ssValue(MakeUCharSpan(slValue), SER_DISK, CLIENT_VERSION);
             ssValue.Xor(dbwrapper_private::GetObfuscateKey(parent));
             ssValue >> value;
         } catch (const std::exception&) {
@@ -243,7 +244,7 @@ class CDBWrapper
             dbwrapper_private::HandleError(status);
         }
         try {
-            CDataStream ssValue(strValue.data(), strValue.data() + strValue.size(), SER_DISK, CLIENT_VERSION);
+            CDataStream ssValue(MakeUCharSpan(strValue), SER_DISK, CLIENT_VERSION);
             ssValue.Xor(obfuscate_key);
             ssValue >> value;
         } catch (const std::exception&) {
@@ -333,7 +334,6 @@ class CDBWrapper
         leveldb::Slice slKey2(ssKey2.data(), ssKey2.size());
         pdb->CompactRange(&slKey1, &slKey2);
     }
-
 };
 
 #endif // BITCOIN_DBWRAPPER_H
diff --git a/src/psbt.cpp b/src/psbt.cpp
@@ -360,7 +360,7 @@ bool DecodeBase64PSBT(PartiallySignedTransaction& psbt, const std::string& base6
 
 bool DecodeRawPSBT(PartiallySignedTransaction& psbt, const std::string& tx_data, std::string& error)
 {
-    CDataStream ss_data(tx_data.data(), tx_data.data() + tx_data.size(), SER_NETWORK, PROTOCOL_VERSION);
+    CDataStream ss_data(MakeUCharSpan(tx_data), SER_NETWORK, PROTOCOL_VERSION);
     try {
         ss_data >> psbt;
         if (!ss_data.empty()) {
diff --git a/src/qt/recentrequeststablemodel.cpp b/src/qt/recentrequeststablemodel.cpp
@@ -179,7 +179,7 @@ void RecentRequestsTableModel::addNewRequest(const SendCoinsRecipient &recipient
 // called from ctor when loading from wallet
 void RecentRequestsTableModel::addNewRequest(const std::string &recipient)
 {
-    std::vector<char> data(recipient.begin(), recipient.end());
+    std::vector<uint8_t> data(recipient.begin(), recipient.end());
     CDataStream ss(data, SER_DISK, CLIENT_VERSION);
 
     RecentRequestEntry entry;
diff --git a/src/streams.h b/src/streams.h
@@ -6,17 +6,18 @@
 #ifndef BITCOIN_STREAMS_H
 #define BITCOIN_STREAMS_H
 
-#include <support/allocators/zeroafterfree.h>
 #include <serialize.h>
+#include <span.h>
+#include <support/allocators/zeroafterfree.h>
 
 #include <algorithm>
 #include <assert.h>
 #include <ios>
 #include <limits>
 #include <stdint.h>
 #include <stdio.h>
-#include <string>
 #include <string.h>
+#include <string>
 #include <utility>
 #include <vector>
 
@@ -225,27 +226,8 @@ class CDataStream
         Init(nTypeIn, nVersionIn);
     }
 
-    CDataStream(const_iterator pbegin, const_iterator pend, int nTypeIn, int nVersionIn) : vch(pbegin, pend)
-    {
-        Init(nTypeIn, nVersionIn);
-    }
-
-    CDataStream(const char* pbegin, const char* pend, int nTypeIn, int nVersionIn) : vch(pbegin, pend)
-    {
-        Init(nTypeIn, nVersionIn);
-    }
-
-    CDataStream(const vector_type& vchIn, int nTypeIn, int nVersionIn) : vch(vchIn.begin(), vchIn.end())
-    {
-        Init(nTypeIn, nVersionIn);
-    }
-
-    CDataStream(const std::vector<char>& vchIn, int nTypeIn, int nVersionIn) : vch(vchIn.begin(), vchIn.end())
-    {
-        Init(nTypeIn, nVersionIn);
-    }
-
-    CDataStream(const std::vector<unsigned char>& vchIn, int nTypeIn, int nVersionIn) : vch(vchIn.begin(), vchIn.end())
+    explicit CDataStream(Span<const uint8_t> sp, int nTypeIn, int nVersionIn)
+        : vch(sp.data(), sp.data() + sp.size())
     {
         Init(nTypeIn, nVersionIn);
     }
@@ -289,7 +271,7 @@ class CDataStream
     value_type* data()                               { return vch.data() + nReadPos; }
     const value_type* data() const                   { return vch.data() + nReadPos; }
 
-    void insert(iterator it, std::vector<char>::const_iterator first, std::vector<char>::const_iterator last)
+    void insert(iterator it, std::vector<uint8_t>::const_iterator first, std::vector<uint8_t>::const_iterator last)
     {
         if (last == first) return;
         assert(last - first > 0);
diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h
@@ -48,7 +48,7 @@ NODISCARD inline std::vector<bool> ConsumeRandomLengthBitVector(FuzzedDataProvid
 
 NODISCARD inline CDataStream ConsumeDataStream(FuzzedDataProvider& fuzzed_data_provider, const size_t max_length = 4096) noexcept
 {
-    return {ConsumeRandomLengthByteVector(fuzzed_data_provider, max_length), SER_NETWORK, INIT_PROTO_VERSION};
+    return CDataStream{ConsumeRandomLengthByteVector(fuzzed_data_provider, max_length), SER_NETWORK, INIT_PROTO_VERSION};
 }
 
 NODISCARD inline std::vector<std::string> ConsumeRandomLengthStringVector(FuzzedDataProvider& fuzzed_data_provider, const size_t max_vector_size = 16, const size_t max_string_length = 16) noexcept
diff --git a/src/test/streams_tests.cpp b/src/test/streams_tests.cpp
@@ -149,7 +149,7 @@ BOOST_AUTO_TEST_CASE(bitstream_reader_writer)
 
 BOOST_AUTO_TEST_CASE(streams_serializedata_xor)
 {
-    std::vector<char> in;
+    std::vector<uint8_t> in;
     std::vector<char> expected_xor;
     std::vector<unsigned char> key;
     CDataStream ds(in, 0, 0);

Original file line number	Diff line number	Diff line change
`@@ -360,7 +360,7 @@ bool DecodeBase64PSBT(PartiallySignedTransaction& psbt, const std::string& base6`
`360`	`360`
`361`	`361`	`bool DecodeRawPSBT(PartiallySignedTransaction& psbt, const std::string& tx_data, std::string& error)`
`362`	`362`	`{`
`363`		`- CDataStream ss_data(tx_data.data(), tx_data.data() + tx_data.size(), SER_NETWORK, PROTOCOL_VERSION);`
	`363`	`+ CDataStream ss_data(MakeUCharSpan(tx_data), SER_NETWORK, PROTOCOL_VERSION);`
`364`	`364`	`try {`
`365`	`365`	`ss_data >> psbt;`
`366`	`366`	`if (!ss_data.empty()) {`
Original file line number	Diff line number	Diff line change
`@@ -179,7 +179,7 @@ void RecentRequestsTableModel::addNewRequest(const SendCoinsRecipient &recipient`
`179`	`179`	`// called from ctor when loading from wallet`
`180`	`180`	`void RecentRequestsTableModel::addNewRequest(const std::string &recipient)`
`181`	`181`	`{`
`182`		`- std::vector<char> data(recipient.begin(), recipient.end());`
	`182`	`+ std::vector<uint8_t> data(recipient.begin(), recipient.end());`
`183`	`183`	`CDataStream ss(data, SER_DISK, CLIENT_VERSION);`
`184`	`184`
`185`	`185`	`RecentRequestEntry entry;`
Original file line number	Diff line number	Diff line change
`@@ -48,7 +48,7 @@ NODISCARD inline std::vector<bool> ConsumeRandomLengthBitVector(FuzzedDataProvid`
`48`	`48`
`49`	`49`	`NODISCARD inline CDataStream ConsumeDataStream(FuzzedDataProvider& fuzzed_data_provider, const size_t max_length = 4096) noexcept`
`50`	`50`	`{`
`51`		`- return {ConsumeRandomLengthByteVector(fuzzed_data_provider, max_length), SER_NETWORK, INIT_PROTO_VERSION};`
	`51`	`+ return CDataStream{ConsumeRandomLengthByteVector(fuzzed_data_provider, max_length), SER_NETWORK, INIT_PROTO_VERSION};`
`52`	`52`	`}`
`53`	`53`
`54`	`54`	`NODISCARD inline std::vector<std::string> ConsumeRandomLengthStringVector(FuzzedDataProvider& fuzzed_data_provider, const size_t max_vector_size = 16, const size_t max_string_length = 16) noexcept`
Original file line number	Diff line number	Diff line change
`@@ -149,7 +149,7 @@ BOOST_AUTO_TEST_CASE(bitstream_reader_writer)`
`149`	`149`
`150`	`150`	`BOOST_AUTO_TEST_CASE(streams_serializedata_xor)`
`151`	`151`	`{`
`152`		`- std::vector<char> in;`
	`152`	`+ std::vector<uint8_t> in;`
`153`	`153`	`std::vector<char> expected_xor;`
`154`	`154`	`std::vector<unsigned char> key;`
`155`	`155`	`CDataStream ds(in, 0, 0);`