From 616be1d76cdb853a1a6fc96a2fbb1cb749943ccc Mon Sep 17 00:00:00 2001 From: Howard Hinnant Date: Mon, 29 Sep 2014 12:48:37 -0400 Subject: [PATCH] Miscellaneous cleanups: * Limit HashPrefix construction and disallow assignment * Give KnownFormats deleted copy members so that derived classes will give the right answers if queried with the std::is_copy_constructible/assignable traits. * Replace SharedSingleton with a local static in LedgerFormats::getInstance() to be consistent with similar code in other places. This also allows the LedgerFormats default constructor to be marked private so that the compiler enforces the design that LedgerFormats is a singleton type. * Change return types of LedgerFormats::getInstance() and TxFormats::getInstance() from pointer to non-const to reference to const so as follow more established design guidelines for singletons. This prevents pointers being mistaken for heap-allocated objects, and the const ensures the singleton isn't mutable. * Change RippleAddress to inherit privately from CBase58Data instead of publicly. This lets the compiler enforce that there are no unintended conversions from RippleAddress to CBase58Data. This change allows us to remove a comment warning about unwanted conversions. --- src/ripple/app/misc/SerializedLedger.cpp | 4 +- src/ripple/app/misc/SerializedTransaction.cpp | 6 +- src/ripple/data/protocol/HashPrefix.h | 5 +- src/ripple/data/protocol/KnownFormats.h | 3 + src/ripple/data/protocol/LedgerFormats.cpp | 6 +- src/ripple/data/protocol/LedgerFormats.h | 5 +- src/ripple/data/protocol/RippleAddress.h | 59 ++++++++++++++++++- src/ripple/data/protocol/STInteger.cpp | 8 +-- src/ripple/data/protocol/STParsedJSON.cpp | 4 +- src/ripple/data/protocol/TxFormats.cpp | 6 +- src/ripple/data/protocol/TxFormats.h | 2 +- 11 files changed, 86 insertions(+), 22 deletions(-) diff --git a/src/ripple/app/misc/SerializedLedger.cpp b/src/ripple/app/misc/SerializedLedger.cpp index 49a5705fd..8dd661179 100644 --- a/src/ripple/app/misc/SerializedLedger.cpp +++ b/src/ripple/app/misc/SerializedLedger.cpp @@ -46,7 +46,7 @@ SerializedLedgerEntry::SerializedLedgerEntry ( void SerializedLedgerEntry::setSLEType () { - mFormat = LedgerFormats::getInstance()->findByType ( + mFormat = LedgerFormats::getInstance().findByType ( static_cast (getFieldU16 (sfLedgerEntryType))); if (mFormat == nullptr) @@ -65,7 +65,7 @@ void SerializedLedgerEntry::setSLEType () SerializedLedgerEntry::SerializedLedgerEntry (LedgerEntryType type, uint256 const& index) : STObject (sfLedgerEntry), mIndex (index), mType (type), mMutable (true) { - mFormat = LedgerFormats::getInstance()->findByType (type); + mFormat = LedgerFormats::getInstance().findByType (type); if (mFormat == nullptr) throw std::runtime_error ("invalid ledger entry type"); diff --git a/src/ripple/app/misc/SerializedTransaction.cpp b/src/ripple/app/misc/SerializedTransaction.cpp index f87731747..6e8419c6d 100644 --- a/src/ripple/app/misc/SerializedTransaction.cpp +++ b/src/ripple/app/misc/SerializedTransaction.cpp @@ -29,7 +29,7 @@ SerializedTransaction::SerializedTransaction (TxType type) , mSigGood (false) , mSigBad (false) { - mFormat = TxFormats::getInstance()->findByType (type); + mFormat = TxFormats::getInstance().findByType (type); if (mFormat == nullptr) { @@ -48,7 +48,7 @@ SerializedTransaction::SerializedTransaction (STObject const& object) { mType = static_cast (getFieldU16 (sfTransactionType)); - mFormat = TxFormats::getInstance()->findByType (mType); + mFormat = TxFormats::getInstance().findByType (mType); if (!mFormat) { @@ -76,7 +76,7 @@ SerializedTransaction::SerializedTransaction (SerializerIterator& sit) : STObjec set (sit); mType = static_cast (getFieldU16 (sfTransactionType)); - mFormat = TxFormats::getInstance()->findByType (mType); + mFormat = TxFormats::getInstance().findByType (mType); if (!mFormat) { diff --git a/src/ripple/data/protocol/HashPrefix.h b/src/ripple/data/protocol/HashPrefix.h index d2a5df3ba..4b4b05cb2 100644 --- a/src/ripple/data/protocol/HashPrefix.h +++ b/src/ripple/data/protocol/HashPrefix.h @@ -42,7 +42,6 @@ class HashPrefix private: std::uint32_t m_prefix; -public: HashPrefix (char a, char b, char c) : m_prefix (0) { @@ -52,6 +51,10 @@ public: m_prefix = m_prefix << 8; } +public: + HashPrefix(HashPrefix const&) = delete; + HashPrefix& operator=(HashPrefix const&) = delete; + /** Returns the hash prefix associated with this object */ operator std::uint32_t () const { diff --git a/src/ripple/data/protocol/KnownFormats.h b/src/ripple/data/protocol/KnownFormats.h index fca5c9dfa..65b1f1f62 100644 --- a/src/ripple/data/protocol/KnownFormats.h +++ b/src/ripple/data/protocol/KnownFormats.h @@ -182,6 +182,9 @@ protected: virtual void addCommonFields (Item& item) = 0; private: + KnownFormats(KnownFormats const&) = delete; + KnownFormats& operator=(KnownFormats const&) = delete; + std::vector > m_formats; NameMap m_names; TypeMap m_types; diff --git a/src/ripple/data/protocol/LedgerFormats.cpp b/src/ripple/data/protocol/LedgerFormats.cpp index 23eb3be86..a904b0db9 100644 --- a/src/ripple/data/protocol/LedgerFormats.cpp +++ b/src/ripple/data/protocol/LedgerFormats.cpp @@ -119,9 +119,11 @@ void LedgerFormats::addCommonFields (Item& item) ; } -LedgerFormats* LedgerFormats::getInstance () +LedgerFormats const& +LedgerFormats::getInstance () { - return beast::SharedSingleton ::getInstance (); + static LedgerFormats instance; + return instance; } } // ripple diff --git a/src/ripple/data/protocol/LedgerFormats.h b/src/ripple/data/protocol/LedgerFormats.h index d929702df..e0bdd29f8 100644 --- a/src/ripple/data/protocol/LedgerFormats.h +++ b/src/ripple/data/protocol/LedgerFormats.h @@ -131,10 +131,11 @@ enum LedgerSpecificFlags */ class LedgerFormats : public KnownFormats { -public: +private: LedgerFormats (); - static LedgerFormats* getInstance (); +public: + static LedgerFormats const& getInstance (); private: void addCommonFields (Item& item); diff --git a/src/ripple/data/protocol/RippleAddress.h b/src/ripple/data/protocol/RippleAddress.h index 2a86719bf..e497ccd86 100644 --- a/src/ripple/data/protocol/RippleAddress.h +++ b/src/ripple/data/protocol/RippleAddress.h @@ -38,9 +38,8 @@ namespace ripple { // Used to hold addresses and parse and produce human formats. // // XXX This needs to be reworked to store data in uint160 and uint256. -// Conversion to CBase58Data should only happen as needed. -class RippleAddress : public CBase58Data +class RippleAddress : private CBase58Data { private: typedef enum @@ -229,10 +228,66 @@ public: static RippleAddress createSeedRandom (); static RippleAddress createSeedGeneric (std::string const& strText); + + std::string ToString () const + {return static_cast(*this).ToString();} + + template + friend + void + hash_append(Hasher& hasher, RippleAddress const& value) + { + using beast::hash_append; + hash_append(hasher, static_cast(value)); + } + + friend + bool + operator==(RippleAddress const& lhs, RippleAddress const& rhs) + { + return static_cast(lhs) == + static_cast(rhs); + } + + friend + bool + operator <(RippleAddress const& lhs, RippleAddress const& rhs) + { + return static_cast(lhs) < + static_cast(rhs); + } }; //------------------------------------------------------------------------------ +inline +bool +operator!=(RippleAddress const& lhs, RippleAddress const& rhs) +{ + return !(lhs == rhs); +} + +inline +bool +operator >(RippleAddress const& lhs, RippleAddress const& rhs) +{ + return rhs < lhs; +} + +inline +bool +operator<=(RippleAddress const& lhs, RippleAddress const& rhs) +{ + return !(rhs < lhs); +} + +inline +bool +operator>=(RippleAddress const& lhs, RippleAddress const& rhs) +{ + return !(lhs < rhs); +} + /** RipplePublicKey */ template <> struct RipplePublicKeyTraits::assign diff --git a/src/ripple/data/protocol/STInteger.cpp b/src/ripple/data/protocol/STInteger.cpp index ad71c871e..cad380ec5 100644 --- a/src/ripple/data/protocol/STInteger.cpp +++ b/src/ripple/data/protocol/STInteger.cpp @@ -85,7 +85,7 @@ std::string STUInt16::getText () const { if (getFName () == sfLedgerEntryType) { - auto item = LedgerFormats::getInstance ()->findByType ( + auto item = LedgerFormats::getInstance ().findByType ( static_cast (value_)); if (item != nullptr) @@ -95,7 +95,7 @@ std::string STUInt16::getText () const if (getFName () == sfTransactionType) { TxFormats::Item const* const item = - TxFormats::getInstance()->findByType (static_cast (value_)); + TxFormats::getInstance().findByType (static_cast (value_)); if (item != nullptr) return item->getName (); @@ -110,7 +110,7 @@ Json::Value STUInt16::getJson (int) const if (getFName () == sfLedgerEntryType) { LedgerFormats::Item const* const item = - LedgerFormats::getInstance ()->findByType (static_cast (value_)); + LedgerFormats::getInstance ().findByType (static_cast (value_)); if (item != nullptr) return item->getName (); @@ -119,7 +119,7 @@ Json::Value STUInt16::getJson (int) const if (getFName () == sfTransactionType) { TxFormats::Item const* const item = - TxFormats::getInstance()->findByType (static_cast (value_)); + TxFormats::getInstance().findByType (static_cast (value_)); if (item != nullptr) return item->getName (); diff --git a/src/ripple/data/protocol/STParsedJSON.cpp b/src/ripple/data/protocol/STParsedJSON.cpp index 21d1b82f9..e4b6a7541 100644 --- a/src/ripple/data/protocol/STParsedJSON.cpp +++ b/src/ripple/data/protocol/STParsedJSON.cpp @@ -204,7 +204,7 @@ bool STParsedJSON::parse (std::string const& json_name, { if (field == sfTransactionType) { - TxType const txType (TxFormats::getInstance()-> + TxType const txType (TxFormats::getInstance(). findTypeByName (strValue)); data.push_back (new STUInt16 (field, @@ -215,7 +215,7 @@ bool STParsedJSON::parse (std::string const& json_name, } else if (field == sfLedgerEntryType) { - LedgerEntryType const type (LedgerFormats::getInstance()-> + LedgerEntryType const type (LedgerFormats::getInstance(). findTypeByName (strValue)); data.push_back (new STUInt16 (field, diff --git a/src/ripple/data/protocol/TxFormats.cpp b/src/ripple/data/protocol/TxFormats.cpp index b859fc6aa..abd6d51d8 100644 --- a/src/ripple/data/protocol/TxFormats.cpp +++ b/src/ripple/data/protocol/TxFormats.cpp @@ -102,11 +102,11 @@ void TxFormats::addCommonFields (Item& item) ; } -TxFormats* TxFormats::getInstance () +TxFormats const& +TxFormats::getInstance () { static TxFormats instance; - //return SharedSingleton ::getInstance (); - return &instance; + return instance; } } // ripple diff --git a/src/ripple/data/protocol/TxFormats.h b/src/ripple/data/protocol/TxFormats.h index d823a3417..5e151f508 100644 --- a/src/ripple/data/protocol/TxFormats.h +++ b/src/ripple/data/protocol/TxFormats.h @@ -64,7 +64,7 @@ public: */ TxFormats (); - static TxFormats* getInstance (); + static TxFormats const& getInstance (); }; } // ripple