From 7e3dbce3d257fbf7c1a8a8437768b3846d753469 Mon Sep 17 00:00:00 2001 From: Edward Hennis Date: Fri, 29 Apr 2016 17:02:48 -0400 Subject: [PATCH] Access base_uint through public members (RIPD-898): * Updates many (but probably not all) locations that access base_uint private storage. * More calls to access base_uint through members. * Use an iterator to write Serializer collections. --- src/ripple/basics/base_uint.h | 11 ++++---- .../crypto/impl/GenerateDeterministicKey.cpp | 4 +-- src/ripple/protocol/Serializer.h | 17 ++++++++++++ src/ripple/protocol/impl/AccountID.cpp | 2 +- src/ripple/protocol/impl/STVector256.cpp | 2 +- src/ripple/protocol/tests/STObject.test.cpp | 2 +- src/ripple/shamap/SHAMapTreeNode.h | 2 +- src/ripple/shamap/impl/SHAMapTreeNode.cpp | 27 ++++++++++--------- 8 files changed, 44 insertions(+), 23 deletions(-) diff --git a/src/ripple/basics/base_uint.h b/src/ripple/basics/base_uint.h index 1a82d12f3c..234b489bc3 100644 --- a/src/ripple/basics/base_uint.h +++ b/src/ripple/basics/base_uint.h @@ -54,10 +54,10 @@ protected: enum { WIDTH = Bits / 32 }; // This is really big-endian in byte order. - // We sometimes use unsigned int for speed. + // We sometimes use std::uint32_t for speed. // NIKB TODO: migrate to std::array - unsigned int pn[WIDTH]; + std::uint32_t pn[WIDTH]; public: //-------------------------------------------------------------------------- @@ -268,7 +268,7 @@ public: { for (int i = WIDTH - 1; i >= 0; --i) { - std::uint32_t prev = pn[i]; + auto prev = pn[i]; pn[i] = hostToBigend (bigendToHost (pn[i]) - 1); if (prev != 0) @@ -320,6 +320,7 @@ public: { unsigned char* pOut = begin (); + assert(sizeof(pn) == bytes); for (int i = 0; i < sizeof (pn); ++i) { auto hi = charUnHex(*psz++); @@ -406,9 +407,9 @@ public: return SetHexExact (str.c_str ()); } - unsigned int size () const + constexpr static std::size_t size () { - return sizeof (pn); + return bytes; } base_uint& operator=(Zero) diff --git a/src/ripple/crypto/impl/GenerateDeterministicKey.cpp b/src/ripple/crypto/impl/GenerateDeterministicKey.cpp index 9b89747105..cf0d0281b8 100644 --- a/src/ripple/crypto/impl/GenerateDeterministicKey.cpp +++ b/src/ripple/crypto/impl/GenerateDeterministicKey.cpp @@ -95,7 +95,7 @@ static bignum generateRootDeterministicKey (uint128 const& seed) copy_uint32 (buf.begin() + 16, seq++); auto root = sha512Half(buf); std::fill (buf.begin(), buf.end(), 0); // security erase - privKey.assign ((unsigned char const*) &root, sizeof (root)); + privKey.assign (root.data(), root.size()); root.zero(); // security erase } while (privKey.is_zero() || privKey >= secp256k1curve.order); @@ -154,7 +154,7 @@ static bignum makeHash (Blob const& pubGen, int seq, bignum const& order) copy_uint32 (buf.begin() + 37, subSeq++); auto root = sha512Half_s(buf); std::fill(buf.begin(), buf.end(), 0); // security erase - result.assign ((unsigned char const*) &root, sizeof (root)); + result.assign (root.data(), root.size()); } while (result.is_zero() || result >= order); diff --git a/src/ripple/protocol/Serializer.h b/src/ripple/protocol/Serializer.h index c8d18943f3..e389b72bee 100644 --- a/src/ripple/protocol/Serializer.h +++ b/src/ripple/protocol/Serializer.h @@ -107,6 +107,8 @@ public: int addVL (Blob const& vector); int addVL (Slice const& slice); + template + int addVL (Iter begin, Iter end, int len); int addVL (const void* ptr, int len); // disassemble functions @@ -280,6 +282,21 @@ private: int addEncoded (int length); }; +template +int Serializer::addVL(Iter begin, Iter end, int len) +{ + int ret = addEncoded(len); + for (; begin != end; ++begin) + { + addRaw(begin->data(), begin->size()); +#ifndef NDEBUG + len -= begin->size(); +#endif + } + assert(len == 0); + return ret; +} + //------------------------------------------------------------------------------ // DEPRECATED diff --git a/src/ripple/protocol/impl/AccountID.cpp b/src/ripple/protocol/impl/AccountID.cpp index 5ca27f03e0..3537c75695 100644 --- a/src/ripple/protocol/impl/AccountID.cpp +++ b/src/ripple/protocol/impl/AccountID.cpp @@ -147,7 +147,7 @@ calcAccountID (PublicKey const& pk) auto const d = static_cast< ripesha_hasher::result_type>(rsh); AccountID id; - static_assert(sizeof(d) == sizeof(id), ""); + static_assert(sizeof(d) == id.size(), ""); std::memcpy(id.data(), d.data(), d.size()); return id; } diff --git a/src/ripple/protocol/impl/STVector256.cpp b/src/ripple/protocol/impl/STVector256.cpp index ba5164df7b..eff4f92461 100644 --- a/src/ripple/protocol/impl/STVector256.cpp +++ b/src/ripple/protocol/impl/STVector256.cpp @@ -48,7 +48,7 @@ STVector256::add (Serializer& s) const { assert (fName->isBinary ()); assert (fName->fieldType == STI_VECTOR256); - s.addVL (mValue.empty () ? nullptr : mValue[0].begin (), mValue.size () * (256 / 8)); + s.addVL (mValue.begin(), mValue.end(), mValue.size () * (256 / 8)); } bool diff --git a/src/ripple/protocol/tests/STObject.test.cpp b/src/ripple/protocol/tests/STObject.test.cpp index c29bdb838d..df3fa5f937 100644 --- a/src/ripple/protocol/tests/STObject.test.cpp +++ b/src/ripple/protocol/tests/STObject.test.cpp @@ -203,7 +203,7 @@ public: auto const& uints1 = object1.getFieldV256(sfTestV256); auto const& uints3 = object3.getFieldV256(sfTestV256); - expect(uints1 == uints3); + expect(uints1 == uints3, "STObject vector mismatch"); } } diff --git a/src/ripple/shamap/SHAMapTreeNode.h b/src/ripple/shamap/SHAMapTreeNode.h index 2039b74b03..c9e15afe2e 100644 --- a/src/ripple/shamap/SHAMapTreeNode.h +++ b/src/ripple/shamap/SHAMapTreeNode.h @@ -147,7 +147,7 @@ class SHAMapInnerNodeV2; class SHAMapInnerNode : public SHAMapAbstractNode { - SHAMapHash mHashes[16]; + std::array mHashes; std::shared_ptr mChildren[16]; int mIsBranch = 0; std::uint32_t mFullBelowGen = 0; diff --git a/src/ripple/shamap/impl/SHAMapTreeNode.cpp b/src/ripple/shamap/impl/SHAMapTreeNode.cpp index e1ba89b76d..e141401158 100644 --- a/src/ripple/shamap/impl/SHAMapTreeNode.cpp +++ b/src/ripple/shamap/impl/SHAMapTreeNode.cpp @@ -43,7 +43,7 @@ SHAMapInnerNode::clone(std::uint32_t seq) const p->mHash = mHash; p->mIsBranch = mIsBranch; p->mFullBelowGen = mFullBelowGen; - std::memcpy(p->mHashes, mHashes, sizeof(mHashes)); + p->mHashes = mHashes; std::unique_lock lock(childLock); for (int i = 0; i < 16; ++i) { @@ -60,7 +60,7 @@ SHAMapInnerNodeV2::clone(std::uint32_t seq) const p->mHash = mHash; p->mIsBranch = mIsBranch; p->mFullBelowGen = mFullBelowGen; - std::memcpy(p->mHashes, mHashes, sizeof(mHashes)); + p->mHashes = mHashes; p->common_ = common_; p->depth_ = depth_; std::unique_lock lock(childLock); @@ -365,10 +365,13 @@ SHAMapInnerNode::updateHash() uint256 nh; if (mIsBranch != 0) { - // VFALCO This code assumes the layout of a base_uint - nh = sha512Half(HashPrefix::innerNode, - Slice(reinterpret_cast(mHashes), - sizeof (mHashes))); + sha512_half_hasher h; + using beast::hash_append; + hash_append(h, HashPrefix::innerNode); + for(auto const& hh : mHashes) + hash_append(h, hh); + nh = static_cast(h); } if (nh == mHash.as_uint256()) return false; @@ -438,15 +441,15 @@ SHAMapInnerNode::addRaw(Serializer& s, SHANodeFormat format) const { s.add32 (HashPrefix::innerNode); - for (int i = 0; i < 16; ++i) - s.add256 (mHashes[i].as_uint256()); + for (auto const& hh : mHashes) + s.add256 (hh.as_uint256()); } else // format == snfWIRE { if (getBranchCount () < 12) { // compressed node - for (int i = 0; i < 16; ++i) + for (int i = 0; i < mHashes.size(); ++i) if (!isEmptyBranch (i)) { s.add256 (mHashes[i].as_uint256()); @@ -457,8 +460,8 @@ SHAMapInnerNode::addRaw(Serializer& s, SHANodeFormat format) const } else { - for (int i = 0; i < 16; ++i) - s.add256 (mHashes[i].as_uint256()); + for (auto const& hh : mHashes) + s.add256 (hh.as_uint256()); s.add8 (2); } @@ -606,7 +609,7 @@ std::string SHAMapInnerNode::getString(const SHAMapNodeID & id) const { std::string ret = SHAMapAbstractNode::getString(id); - for (int i = 0; i < 16; ++i) + for (int i = 0; i < mHashes.size(); ++i) { if (!isEmptyBranch (i)) {