From a7c45bce9264f4a8fa48f2e7ecaf102971fc026f Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 20 Mar 2018 17:37:32 -0700 Subject: [PATCH] Add native support for serializing char arrays without FLATDATA Support is added to serialize arrays of type char or unsigned char directly, without any wrappers. All invocations of the FLATDATA wrappers that are obsoleted by this are removed. This includes a patch by Russell Yanofsky to make char casting type safe. The serialization of CSubNet is changed to serialize a bool directly rather than though FLATDATA. This makes the serialization independent of the size of the bool type (and will use 1 byte everywhere). --- src/addrdb.cpp | 6 +++--- src/netaddress.h | 8 ++++---- src/protocol.h | 6 +++--- src/serialize.h | 10 ++++++++++ src/test/net_tests.cpp | 6 +++--- src/test/serialize_tests.cpp | 16 ++++++++++------ src/test/streams_tests.cpp | 8 ++++---- src/validation.cpp | 6 +++--- 8 files changed, 40 insertions(+), 26 deletions(-) diff --git a/src/addrdb.cpp b/src/addrdb.cpp index 675f3c28a..e4620e63c 100644 --- a/src/addrdb.cpp +++ b/src/addrdb.cpp @@ -22,8 +22,8 @@ bool SerializeDB(Stream& stream, const Data& data) // Write and commit header, data try { CHashWriter hasher(SER_DISK, CLIENT_VERSION); - stream << FLATDATA(Params().MessageStart()) << data; - hasher << FLATDATA(Params().MessageStart()) << data; + stream << Params().MessageStart() << data; + hasher << Params().MessageStart() << data; stream << hasher.GetHash(); } catch (const std::exception& e) { return error("%s: Serialize or I/O error - %s", __func__, e.what()); @@ -66,7 +66,7 @@ bool DeserializeDB(Stream& stream, Data& data, bool fCheckSum = true) CHashVerifier verifier(&stream); // de-serialize file header (network specific magic number) and .. unsigned char pchMsgTmp[4]; - verifier >> FLATDATA(pchMsgTmp); + verifier >> pchMsgTmp; // ... verify the network matches ours if (memcmp(pchMsgTmp, Params().MessageStart(), sizeof(pchMsgTmp))) return error("%s: Invalid network magic number", __func__); diff --git a/src/netaddress.h b/src/netaddress.h index 93bbb6649..ad6b55eb5 100644 --- a/src/netaddress.h +++ b/src/netaddress.h @@ -93,7 +93,7 @@ class CNetAddr template inline void SerializationOp(Stream& s, Operation ser_action) { - READWRITE(FLATDATA(ip)); + READWRITE(ip); } friend class CSubNet; @@ -131,8 +131,8 @@ class CSubNet template inline void SerializationOp(Stream& s, Operation ser_action) { READWRITE(network); - READWRITE(FLATDATA(netmask)); - READWRITE(FLATDATA(valid)); + READWRITE(netmask); + READWRITE(valid); } }; @@ -166,7 +166,7 @@ class CService : public CNetAddr template inline void SerializationOp(Stream& s, Operation ser_action) { - READWRITE(FLATDATA(ip)); + READWRITE(ip); unsigned short portN = htons(port); READWRITE(FLATDATA(portN)); if (ser_action.ForRead()) diff --git a/src/protocol.h b/src/protocol.h index e518d1194..a07c5ea86 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -48,10 +48,10 @@ public: template inline void SerializationOp(Stream& s, Operation ser_action) { - READWRITE(FLATDATA(pchMessageStart)); - READWRITE(FLATDATA(pchCommand)); + READWRITE(pchMessageStart); + READWRITE(pchCommand); READWRITE(nMessageSize); - READWRITE(FLATDATA(pchChecksum)); + READWRITE(pchChecksum); } char pchMessageStart[MESSAGE_START_SIZE]; diff --git a/src/serialize.h b/src/serialize.h index 91da6b0f8..247e91529 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -59,6 +59,12 @@ inline T* NCONST_PTR(const T* val) return const_cast(val); } +//! Safely convert odd char pointer types to standard ones. +inline char* CharCast(char* c) { return c; } +inline char* CharCast(unsigned char* c) { return (char*)c; } +inline const char* CharCast(const char* c) { return c; } +inline const char* CharCast(const unsigned char* c) { return (const char*)c; } + /* * Lowest-level serialization and conversion. * @note Sizes of these types are verified in the tests @@ -177,6 +183,8 @@ 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, float a ) { ser_writedata32(s, ser_float_to_uint32(a)); } template inline void Serialize(Stream& s, double a ) { ser_writedata64(s, ser_double_to_uint64(a)); } +template inline void Serialize(Stream& s, const char (&a)[N]) { s.write(a, N); } +template inline void Serialize(Stream& s, const unsigned char (&a)[N]) { s.write(CharCast(a), N); } template inline void Unserialize(Stream& s, char& a ) { a = ser_readdata8(s); } // TODO Get rid of bare char template inline void Unserialize(Stream& s, int8_t& a ) { a = ser_readdata8(s); } @@ -189,6 +197,8 @@ 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, float& a ) { a = ser_uint32_to_float(ser_readdata32(s)); } template inline void Unserialize(Stream& s, double& a ) { a = ser_uint64_to_double(ser_readdata64(s)); } +template inline void Unserialize(Stream& s, char (&a)[N]) { s.read(a, N); } +template inline void Unserialize(Stream& s, unsigned char (&a)[N]) { s.read(CharCast(a), N); } template inline void Serialize(Stream& s, bool a) { char f=a; ser_writedata8(s, f); } template inline void Unserialize(Stream& s, bool& a) { char f=ser_readdata8(s); a=f; } diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index e03234060..6552613c0 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -64,7 +64,7 @@ public: CDataStream AddrmanToStream(CAddrManSerializationMock& _addrman) { CDataStream ssPeersIn(SER_DISK, CLIENT_VERSION); - ssPeersIn << FLATDATA(Params().MessageStart()); + ssPeersIn << Params().MessageStart(); ssPeersIn << _addrman; std::string str = ssPeersIn.str(); std::vector vchData(str.begin(), str.end()); @@ -110,7 +110,7 @@ BOOST_AUTO_TEST_CASE(caddrdb_read) BOOST_CHECK(addrman1.size() == 0); try { unsigned char pchMsgTmp[4]; - ssPeers1 >> FLATDATA(pchMsgTmp); + ssPeers1 >> pchMsgTmp; ssPeers1 >> addrman1; } catch (const std::exception& e) { exceptionThrown = true; @@ -142,7 +142,7 @@ BOOST_AUTO_TEST_CASE(caddrdb_read_corrupted) BOOST_CHECK(addrman1.size() == 0); try { unsigned char pchMsgTmp[4]; - ssPeers1 >> FLATDATA(pchMsgTmp); + ssPeers1 >> pchMsgTmp; ssPeers1 >> addrman1; } catch (const std::exception& e) { exceptionThrown = true; diff --git a/src/test/serialize_tests.cpp b/src/test/serialize_tests.cpp index 7a79a77e8..9b8b7bdc5 100644 --- a/src/test/serialize_tests.cpp +++ b/src/test/serialize_tests.cpp @@ -19,11 +19,15 @@ protected: int intval; bool boolval; std::string stringval; - const char* charstrval; + char charstrval[16]; CTransactionRef txval; public: CSerializeMethodsTestSingle() = default; - CSerializeMethodsTestSingle(int intvalin, bool boolvalin, std::string stringvalin, const char* charstrvalin, CTransaction txvalin) : intval(intvalin), boolval(boolvalin), stringval(std::move(stringvalin)), charstrval(charstrvalin), txval(MakeTransactionRef(txvalin)){} + CSerializeMethodsTestSingle(int intvalin, bool boolvalin, std::string stringvalin, const char* charstrvalin, CTransaction txvalin) : intval(intvalin), boolval(boolvalin), stringval(std::move(stringvalin)), txval(MakeTransactionRef(txvalin)) + { + memcpy(charstrval, charstrvalin, sizeof(charstrval)); + } + ADD_SERIALIZE_METHODS; template @@ -31,7 +35,7 @@ public: READWRITE(intval); READWRITE(boolval); READWRITE(stringval); - READWRITE(FLATDATA(charstrval)); + READWRITE(charstrval); READWRITE(txval); } @@ -53,7 +57,7 @@ public: template inline void SerializationOp(Stream& s, Operation ser_action) { - READWRITE(intval, boolval, stringval, FLATDATA(charstrval), txval); + READWRITE(intval, boolval, stringval, charstrval, txval); } }; @@ -344,7 +348,7 @@ BOOST_AUTO_TEST_CASE(class_methods) int intval(100); bool boolval(true); std::string stringval("testing"); - const char* charstrval("testing charstr"); + const char charstrval[16] = "testing charstr"; CMutableTransaction txval; CSerializeMethodsTestSingle methodtest1(intval, boolval, stringval, charstrval, txval); CSerializeMethodsTestMany methodtest2(intval, boolval, stringval, charstrval, txval); @@ -360,7 +364,7 @@ BOOST_AUTO_TEST_CASE(class_methods) BOOST_CHECK(methodtest2 == methodtest3); BOOST_CHECK(methodtest3 == methodtest4); - CDataStream ss2(SER_DISK, PROTOCOL_VERSION, intval, boolval, stringval, FLATDATA(charstrval), txval); + CDataStream ss2(SER_DISK, PROTOCOL_VERSION, intval, boolval, stringval, charstrval, txval); ss2 >> methodtest3; BOOST_CHECK(methodtest3 == methodtest4); } diff --git a/src/test/streams_tests.cpp b/src/test/streams_tests.cpp index 1108dab58..5d057108b 100644 --- a/src/test/streams_tests.cpp +++ b/src/test/streams_tests.cpp @@ -57,16 +57,16 @@ BOOST_AUTO_TEST_CASE(streams_vector_writer) BOOST_CHECK((vch == std::vector{{0, 0, 0, 0, 1, 2}})); vch.clear(); - CVectorWriter(SER_NETWORK, INIT_PROTO_VERSION, vch, 0, FLATDATA(bytes)); + CVectorWriter(SER_NETWORK, INIT_PROTO_VERSION, vch, 0, bytes); BOOST_CHECK((vch == std::vector{{3, 4, 5, 6}})); - CVectorWriter(SER_NETWORK, INIT_PROTO_VERSION, vch, 0, FLATDATA(bytes)); + CVectorWriter(SER_NETWORK, INIT_PROTO_VERSION, vch, 0, bytes); BOOST_CHECK((vch == std::vector{{3, 4, 5, 6}})); vch.clear(); vch.resize(4, 8); - CVectorWriter(SER_NETWORK, INIT_PROTO_VERSION, vch, 2, a, FLATDATA(bytes), b); + CVectorWriter(SER_NETWORK, INIT_PROTO_VERSION, vch, 2, a, bytes, b); BOOST_CHECK((vch == std::vector{{8, 8, 1, 3, 4, 5, 6, 2}})); - CVectorWriter(SER_NETWORK, INIT_PROTO_VERSION, vch, 2, a, FLATDATA(bytes), b); + CVectorWriter(SER_NETWORK, INIT_PROTO_VERSION, vch, 2, a, bytes, b); BOOST_CHECK((vch == std::vector{{8, 8, 1, 3, 4, 5, 6, 2}})); vch.clear(); } diff --git a/src/validation.cpp b/src/validation.cpp index bee890437..368caef62 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1077,7 +1077,7 @@ static bool WriteBlockToDisk(const CBlock& block, CDiskBlockPos& pos, const CMes // Write index header unsigned int nSize = GetSerializeSize(fileout, block); - fileout << FLATDATA(messageStart) << nSize; + fileout << messageStart << nSize; // Write block long fileOutPos = ftell(fileout.Get()); @@ -1441,7 +1441,7 @@ bool UndoWriteToDisk(const CBlockUndo& blockundo, CDiskBlockPos& pos, const uint // Write index header unsigned int nSize = GetSerializeSize(fileout, blockundo); - fileout << FLATDATA(messageStart) << nSize; + fileout << messageStart << nSize; // Write undo data long fileOutPos = ftell(fileout.Get()); @@ -4283,7 +4283,7 @@ bool LoadExternalBlockFile(const CChainParams& chainparams, FILE* fileIn, CDiskB unsigned char buf[CMessageHeader::MESSAGE_START_SIZE]; blkdat.FindByte(chainparams.MessageStart()[0]); nRewind = blkdat.GetPos()+1; - blkdat >> FLATDATA(buf); + blkdat >> buf; if (memcmp(buf, chainparams.MessageStart(), CMessageHeader::MESSAGE_START_SIZE)) continue; // read size