From fa257bc8312b91c2d281f48ca2500d9cba353cc5 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Wed, 21 Jun 2023 10:13:08 +0200 Subject: [PATCH 1/2] util: Allow std::byte and char Span serialization --- src/serialize.h | 7 ++++--- src/test/serialize_tests.cpp | 9 +++++++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/serialize.h b/src/serialize.h index 7bc7b10779c..348a6ae4f1a 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -188,6 +188,7 @@ template const X& ReadWriteAsHelper(const X& x) { return x; } } \ FORMATTER_METHODS(cls, obj) +// clang-format off #ifndef CHAR_EQUALS_INT8 template void Serialize(Stream&, char) = delete; // char serialization forbidden. Use uint8_t or int8_t #endif @@ -201,8 +202,7 @@ template inline void Serialize(Stream& s, int64_t a ) { ser_wri template inline void Serialize(Stream& s, uint64_t a) { ser_writedata64(s, a); } template inline void Serialize(Stream& s, const char (&a)[N]) { s.write(MakeByteSpan(a)); } template inline void Serialize(Stream& s, const unsigned char (&a)[N]) { s.write(MakeByteSpan(a)); } -template inline void Serialize(Stream& s, const Span& span) { s.write(AsBytes(span)); } -template inline void Serialize(Stream& s, const Span& span) { s.write(AsBytes(span)); } +template void Serialize(Stream& s, Span span) { (void)/* force byte-type */UCharCast(span.data()); s.write(AsBytes(span)); } #ifndef CHAR_EQUALS_INT8 template void Unserialize(Stream&, char) = delete; // char serialization forbidden. Use uint8_t or int8_t @@ -217,10 +217,11 @@ template inline void Unserialize(Stream& s, int64_t& a ) { a = template inline void Unserialize(Stream& s, uint64_t& a) { a = ser_readdata64(s); } template inline void Unserialize(Stream& s, char (&a)[N]) { s.read(MakeWritableByteSpan(a)); } template inline void Unserialize(Stream& s, unsigned char (&a)[N]) { s.read(MakeWritableByteSpan(a)); } -template inline void Unserialize(Stream& s, Span& span) { s.read(AsWritableBytes(span)); } +template void Unserialize(Stream& s, Span span) { (void)/* force byte-type */UCharCast(span.data()); s.read(AsWritableBytes(span)); } template inline void Serialize(Stream& s, bool a) { uint8_t f = a; ser_writedata8(s, f); } template inline void Unserialize(Stream& s, bool& a) { uint8_t f = ser_readdata8(s); a = f; } +// clang-format on /** diff --git a/src/test/serialize_tests.cpp b/src/test/serialize_tests.cpp index 09f77d2b611..88543a8aef5 100644 --- a/src/test/serialize_tests.cpp +++ b/src/test/serialize_tests.cpp @@ -241,6 +241,15 @@ BOOST_AUTO_TEST_CASE(class_methods) ss2 << intval << boolval << stringval << charstrval << txval; ss2 >> methodtest3; BOOST_CHECK(methodtest3 == methodtest4); + { + DataStream ds; + const std::string in{"ab"}; + ds << Span{in}; + std::array out; + ds >> Span{out}; + BOOST_CHECK_EQUAL(out.at(0), std::byte{'a'}); + BOOST_CHECK_EQUAL(out.at(1), std::byte{'b'}); + } } BOOST_AUTO_TEST_SUITE_END() From fa38d862358b87219b12bf31236c52f28d9fc5d6 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Thu, 22 Jun 2023 17:02:28 +0200 Subject: [PATCH 2/2] Use only Span{} constructor for byte-like types where possible This removes bloat that is not needed. --- src/bench/load_external.cpp | 2 +- src/net.cpp | 4 ++-- src/pubkey.h | 4 ++-- src/test/fuzz/p2p_transport_serialization.cpp | 2 +- src/test/serialize_tests.cpp | 14 +++++++------- src/uint256.h | 2 +- src/wallet/dump.cpp | 12 ++++++------ src/wallet/test/db_tests.cpp | 2 +- src/wallet/wallet.cpp | 2 +- src/wallet/walletdb.cpp | 2 +- 10 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/bench/load_external.cpp b/src/bench/load_external.cpp index 2ff72a30121..1378a7b20a5 100644 --- a/src/bench/load_external.cpp +++ b/src/bench/load_external.cpp @@ -34,7 +34,7 @@ static void LoadExternalBlockFile(benchmark::Bench& bench) ss << static_cast(benchmark::data::block413567.size()); // We can't use the streaming serialization (ss << benchmark::data::block413567) // because that first writes a compact size. - ss.write(MakeByteSpan(benchmark::data::block413567)); + ss << Span{benchmark::data::block413567}; // Create the test file. { diff --git a/src/net.cpp b/src/net.cpp index a96ffcfbe99..2cb69d5bfd9 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2939,13 +2939,13 @@ void CaptureMessageToFile(const CAddress& addr, AutoFile f{fsbridge::fopen(path, "ab")}; ser_writedata64(f, now.count()); - f.write(MakeByteSpan(msg_type)); + f << Span{msg_type}; for (auto i = msg_type.length(); i < CMessageHeader::COMMAND_SIZE; ++i) { f << uint8_t{'\0'}; } uint32_t size = data.size(); ser_writedata32(f, size); - f.write(AsBytes(data)); + f << data; } std::function void Unserialize(Stream& s) { const unsigned int len(::ReadCompactSize(s)); if (len <= SIZE) { - s.read(AsWritableBytes(Span{vch, len})); + s >> Span{vch, len}; if (len != size()) { Invalidate(); } diff --git a/src/test/fuzz/p2p_transport_serialization.cpp b/src/test/fuzz/p2p_transport_serialization.cpp index ec3cdbff5a4..a6fe3037e6c 100644 --- a/src/test/fuzz/p2p_transport_serialization.cpp +++ b/src/test/fuzz/p2p_transport_serialization.cpp @@ -77,7 +77,7 @@ FUZZ_TARGET_INIT(p2p_transport_serialization, initialize_p2p_transport_serializa assert(msg.m_time == m_time); std::vector header; - auto msg2 = CNetMsgMaker{msg.m_recv.GetVersion()}.Make(msg.m_type, MakeUCharSpan(msg.m_recv)); + auto msg2 = CNetMsgMaker{msg.m_recv.GetVersion()}.Make(msg.m_type, Span{msg.m_recv}); serializer.prepareForTransport(msg2, header); } } diff --git a/src/test/serialize_tests.cpp b/src/test/serialize_tests.cpp index 88543a8aef5..b445ff8ffcd 100644 --- a/src/test/serialize_tests.cpp +++ b/src/test/serialize_tests.cpp @@ -186,32 +186,32 @@ BOOST_AUTO_TEST_CASE(noncanonical) std::vector::size_type n; // zero encoded with three bytes: - ss.write(MakeByteSpan("\xfd\x00\x00").first(3)); + ss << Span{"\xfd\x00\x00"}.first(3); BOOST_CHECK_EXCEPTION(ReadCompactSize(ss), std::ios_base::failure, isCanonicalException); // 0xfc encoded with three bytes: - ss.write(MakeByteSpan("\xfd\xfc\x00").first(3)); + ss << Span{"\xfd\xfc\x00"}.first(3); BOOST_CHECK_EXCEPTION(ReadCompactSize(ss), std::ios_base::failure, isCanonicalException); // 0xfd encoded with three bytes is OK: - ss.write(MakeByteSpan("\xfd\xfd\x00").first(3)); + ss << Span{"\xfd\xfd\x00"}.first(3); n = ReadCompactSize(ss); BOOST_CHECK(n == 0xfd); // zero encoded with five bytes: - ss.write(MakeByteSpan("\xfe\x00\x00\x00\x00").first(5)); + ss << Span{"\xfe\x00\x00\x00\x00"}.first(5); BOOST_CHECK_EXCEPTION(ReadCompactSize(ss), std::ios_base::failure, isCanonicalException); // 0xffff encoded with five bytes: - ss.write(MakeByteSpan("\xfe\xff\xff\x00\x00").first(5)); + ss << Span{"\xfe\xff\xff\x00\x00"}.first(5); BOOST_CHECK_EXCEPTION(ReadCompactSize(ss), std::ios_base::failure, isCanonicalException); // zero encoded with nine bytes: - ss.write(MakeByteSpan("\xff\x00\x00\x00\x00\x00\x00\x00\x00").first(9)); + ss << Span{"\xff\x00\x00\x00\x00\x00\x00\x00\x00"}.first(9); BOOST_CHECK_EXCEPTION(ReadCompactSize(ss), std::ios_base::failure, isCanonicalException); // 0x01ffffff encoded with nine bytes: - ss.write(MakeByteSpan("\xff\xff\xff\xff\x01\x00\x00\x00\x00").first(9)); + ss << Span{"\xff\xff\xff\xff\x01\x00\x00\x00\x00"}.first(9); BOOST_CHECK_EXCEPTION(ReadCompactSize(ss), std::ios_base::failure, isCanonicalException); } diff --git a/src/uint256.h b/src/uint256.h index 1cc3721487e..135907dcf78 100644 --- a/src/uint256.h +++ b/src/uint256.h @@ -77,7 +77,7 @@ public: template void Serialize(Stream& s) const { - s.write(MakeByteSpan(m_data)); + s << Span(m_data); } template diff --git a/src/wallet/dump.cpp b/src/wallet/dump.cpp index 44c93eed7c2..3ac5cf03b19 100644 --- a/src/wallet/dump.cpp +++ b/src/wallet/dump.cpp @@ -57,12 +57,12 @@ bool DumpWallet(const ArgsManager& args, CWallet& wallet, bilingual_str& error) // Write out a magic string with version std::string line = strprintf("%s,%u\n", DUMP_MAGIC, DUMP_VERSION); dump_file.write(line.data(), line.size()); - hasher.write(MakeByteSpan(line)); + hasher << Span{line}; // Write out the file format line = strprintf("%s,%s\n", "format", db.Format()); dump_file.write(line.data(), line.size()); - hasher.write(MakeByteSpan(line)); + hasher << Span{line}; if (ret) { @@ -83,7 +83,7 @@ bool DumpWallet(const ArgsManager& args, CWallet& wallet, bilingual_str& error) std::string value_str = HexStr(ss_value); line = strprintf("%s,%s\n", key_str, value_str); dump_file.write(line.data(), line.size()); - hasher.write(MakeByteSpan(line)); + hasher << Span{line}; } } @@ -160,7 +160,7 @@ bool CreateFromDump(const ArgsManager& args, const std::string& name, const fs:: return false; } std::string magic_hasher_line = strprintf("%s,%s\n", magic_key, version_value); - hasher.write(MakeByteSpan(magic_hasher_line)); + hasher << Span{magic_hasher_line}; // Get the stored file format std::string format_key; @@ -191,7 +191,7 @@ bool CreateFromDump(const ArgsManager& args, const std::string& name, const fs:: warnings.push_back(strprintf(_("Warning: Dumpfile wallet format \"%s\" does not match command line specified format \"%s\"."), format_value, file_format)); } std::string format_hasher_line = strprintf("%s,%s\n", format_key, format_value); - hasher.write(MakeByteSpan(format_hasher_line)); + hasher << Span{format_hasher_line}; DatabaseOptions options; DatabaseStatus status; @@ -236,7 +236,7 @@ bool CreateFromDump(const ArgsManager& args, const std::string& name, const fs:: } std::string line = strprintf("%s,%s\n", key, value); - hasher.write(MakeByteSpan(line)); + hasher << Span{line}; if (key.empty() || value.empty()) { continue; diff --git a/src/wallet/test/db_tests.cpp b/src/wallet/test/db_tests.cpp index e0b4a1e74c3..d341e84d9b5 100644 --- a/src/wallet/test/db_tests.cpp +++ b/src/wallet/test/db_tests.cpp @@ -196,7 +196,7 @@ BOOST_AUTO_TEST_CASE(db_cursor_prefix_byte_test) for (const auto& database : TestDatabases(m_path_root)) { std::unique_ptr batch = database->MakeBatch(); for (const auto& [k, v] : {e, p, ps, f, fs, ff, ffs}) { - batch->Write(MakeUCharSpan(k), MakeUCharSpan(v)); + batch->Write(Span{k}, Span{v}); } CheckPrefix(*batch, StringBytes(""), {e, p, ps, f, fs, ff, ffs}); CheckPrefix(*batch, StringBytes("prefix"), {p, ps}); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index ba11933b911..62b020dacc6 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3878,7 +3878,7 @@ bool CWallet::MigrateToSQLite(bilingual_str& error) bool began = batch->TxnBegin(); assert(began); // This is a critical error, the new db could not be written to. The original db exists as a backup, but we should not continue execution. for (const auto& [key, value] : records) { - if (!batch->Write(MakeUCharSpan(key), MakeUCharSpan(value))) { + if (!batch->Write(Span{key}, Span{value})) { batch->TxnAbort(); m_database->Close(); fs::remove(m_database->Filename()); diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 34fe8ab17f4..afefedef50d 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -1161,7 +1161,7 @@ bool WalletBatch::EraseRecords(const std::unordered_set& types) } // Make a copy of key to avoid data being deleted by the following read of the type - Span key_data = MakeUCharSpan(key); + Span key_data{key}; std::string type; key >> type;