diff --git a/src/ripple/app/tests/common_ledger.cpp b/src/ripple/app/tests/common_ledger.cpp index 8c932905f..2a98c7b3a 100644 --- a/src/ripple/app/tests/common_ledger.cpp +++ b/src/ripple/app/tests/common_ledger.cpp @@ -112,12 +112,11 @@ STTx parseTransaction(TestAccount& account, Json::Value const& tx_json, bool sign) { STParsedJSONObject parsed("tx_json", tx_json); - std::unique_ptr sopTrans = std::move(parsed.object); - if (sopTrans == nullptr) + if (!parsed.object) throw std::runtime_error( - "sopTrans == nullptr"); - sopTrans->setFieldVL(sfSigningPubKey, account.pk.getAccountPublic()); - auto tx = STTx(*sopTrans); + "object not parseable"); + parsed.object->setFieldVL(sfSigningPubKey, account.pk.getAccountPublic()); + auto tx = STTx(std::move (*parsed.object)); if (sign) tx.sign(account.sk); return tx; diff --git a/src/ripple/protocol/STArray.h b/src/ripple/protocol/STArray.h index f4af447f0..8dc3d3fc6 100644 --- a/src/ripple/protocol/STArray.h +++ b/src/ripple/protocol/STArray.h @@ -71,11 +71,37 @@ public: return emplace(n, buf, std::move(*this)); } - void push_back (const STObject & object) + STObject& operator[] (std::size_t j) + { + return v_[j]; + } + + STObject const& operator[] (std::size_t j) const + { + return v_[j]; + } + + STObject& back() { return v_.back(); } + + STObject const& back() const { return v_.back(); } + + template + void + emplace_back(Args&&... args) + { + v_.emplace_back(std::forward(args)...); + } + + void push_back (STObject const& object) { v_.push_back(object); } + void push_back (STObject&& object) + { + v_.push_back(std::move(object)); + } + iterator begin () { return v_.begin (); @@ -101,11 +127,6 @@ public: return v_.size (); } - STObject& back () - { - return v_.back (); - } - bool empty () const { return v_.empty (); diff --git a/src/ripple/protocol/STObject.h b/src/ripple/protocol/STObject.h index 1032ba367..f60848f96 100644 --- a/src/ripple/protocol/STObject.h +++ b/src/ripple/protocol/STObject.h @@ -27,7 +27,6 @@ #include #include #include -#include #include #include @@ -99,7 +98,6 @@ public: STObject(STObject const&) = default; STObject (const SOTemplate & type, SField const& name); STObject (const SOTemplate & type, SerialIter & sit, SField const& name); - STObject (SField const& name, boost::ptr_vector& data); STObject (SerialIter& sit, SField const& name); STObject& operator= (STObject const&) = default; STObject& operator= (STObject&& other); diff --git a/src/ripple/protocol/STParsedJSON.h b/src/ripple/protocol/STParsedJSON.h index 211a7cc09..c4de51399 100644 --- a/src/ripple/protocol/STParsedJSON.h +++ b/src/ripple/protocol/STParsedJSON.h @@ -21,6 +21,7 @@ #define RIPPLE_PROTOCOL_STPARSEDJSON_H_INCLUDED #include +#include namespace ripple { @@ -45,7 +46,7 @@ public: ~STParsedJSONObject () = default; /** The STObject if the parse was successful. */ - std::unique_ptr object; + boost::optional object; /** On failure, an appropriate set of error values. */ Json::Value error; @@ -72,7 +73,7 @@ public: ~STParsedJSONArray () = default; /** The STArray if the parse was successful. */ - std::unique_ptr array; + boost::optional array; /** On failure, an appropriate set of error values. */ Json::Value error; diff --git a/src/ripple/protocol/STTx.h b/src/ripple/protocol/STTx.h index 3c4999664..a0f1718d1 100644 --- a/src/ripple/protocol/STTx.h +++ b/src/ripple/protocol/STTx.h @@ -53,8 +53,7 @@ public: explicit STTx (SerialIter& sit); explicit STTx (TxType type); - // Only called from ripple::RPC::transactionSign - can we eliminate this? - explicit STTx (STObject const& object); + explicit STTx (STObject&& object); STBase* copy (std::size_t n, void* buf) const override diff --git a/src/ripple/protocol/impl/STObject.cpp b/src/ripple/protocol/impl/STObject.cpp index c2e22a675..97efb2bad 100644 --- a/src/ripple/protocol/impl/STObject.cpp +++ b/src/ripple/protocol/impl/STObject.cpp @@ -71,16 +71,6 @@ STObject::STObject (SOTemplate const& type, setType (type); } -STObject::STObject (SField const& name, - boost::ptr_vector& data) - : STBase (name) - , mType (nullptr) -{ - v_.reserve(data.size()); - for (auto const& b : data) - v_.emplace_back(b); -} - STObject::STObject (SerialIter& sit, SField const& name) : STBase(name) , mType(nullptr) diff --git a/src/ripple/protocol/impl/STParsedJSON.cpp b/src/ripple/protocol/impl/STParsedJSON.cpp index 75a895525..92ad92700 100644 --- a/src/ripple/protocol/impl/STParsedJSON.cpp +++ b/src/ripple/protocol/impl/STParsedJSON.cpp @@ -31,6 +31,7 @@ #include #include #include +#include #include #include #include // @@ -140,14 +141,14 @@ static Json::Value singleton_expected (std::string const& object, // This function is used by parseObject to parse any JSON type that doesn't // recurse. Everything represented here is a leaf-type. -static std::unique_ptr parseLeaf ( +static boost::optional parseLeaf ( std::string const& json_name, std::string const& fieldName, SField const* name, Json::Value const& value, Json::Value& error) { - std::unique_ptr ret; + boost::optional ret; auto const& field = SField::getField (fieldName); @@ -174,7 +175,7 @@ static std::unique_ptr parseLeaf ( return ret; } - ret = std::make_unique (field, + ret = detail::make_stvar (field, range_check_cast ( value.asInt (), 0, 255)); } @@ -186,7 +187,7 @@ static std::unique_ptr parseLeaf ( return ret; } - ret = std::make_unique (field, + ret = detail::make_stvar (field, range_check_cast ( value.asUInt (), 0, 255)); } @@ -218,7 +219,7 @@ static std::unique_ptr parseLeaf ( TxType const txType (TxFormats::getInstance(). findTypeByName (strValue)); - ret = std::make_unique (field, + ret = detail::make_stvar (field, static_cast (txType)); if (*name == sfGeneric) @@ -230,7 +231,7 @@ static std::unique_ptr parseLeaf ( LedgerFormats::getInstance(). findTypeByName (strValue)); - ret = std::make_unique (field, + ret = detail::make_stvar (field, static_cast (type)); if (*name == sfGeneric) @@ -244,19 +245,19 @@ static std::unique_ptr parseLeaf ( } else { - ret = std::make_unique (field, + ret = detail::make_stvar (field, beast::lexicalCastThrow (strValue)); } } else if (value.isInt ()) { - ret = std::make_unique (field, + ret = detail::make_stvar (field, range_check_cast ( value.asInt (), 0, 65535)); } else if (value.isUInt ()) { - ret = std::make_unique (field, + ret = detail::make_stvar (field, range_check_cast ( value.asUInt (), 0, 65535)); } @@ -279,19 +280,19 @@ static std::unique_ptr parseLeaf ( { if (value.isString ()) { - ret = std::make_unique (field, + ret = detail::make_stvar (field, beast::lexicalCastThrow ( value.asString ())); } else if (value.isInt ()) { - ret = std::make_unique (field, + ret = detail::make_stvar (field, range_check_cast ( value.asInt (), 0u, 4294967295u)); } else if (value.isUInt ()) { - ret = std::make_unique (field, + ret = detail::make_stvar (field, static_cast (value.asUInt ())); } else @@ -313,18 +314,18 @@ static std::unique_ptr parseLeaf ( { if (value.isString ()) { - ret = std::make_unique (field, + ret = detail::make_stvar (field, uintFromHex (value.asString ())); } else if (value.isInt ()) { - ret = std::make_unique (field, + ret = detail::make_stvar (field, range_check_cast ( value.asInt (), 0, 18446744073709551615ull)); } else if (value.isUInt ()) { - ret = std::make_unique (field, + ret = detail::make_stvar (field, static_cast (value.asUInt ())); } else @@ -346,7 +347,7 @@ static std::unique_ptr parseLeaf ( { if (value.isString ()) { - ret = std::make_unique (field, value.asString ()); + ret = detail::make_stvar (field, value.asString ()); } else { @@ -367,7 +368,7 @@ static std::unique_ptr parseLeaf ( { if (value.isString ()) { - ret = std::make_unique (field, value.asString ()); + ret = detail::make_stvar (field, value.asString ()); } else { @@ -388,7 +389,7 @@ static std::unique_ptr parseLeaf ( { if (value.isString ()) { - ret = std::make_unique (field, value.asString ()); + ret = detail::make_stvar (field, value.asString ()); } else { @@ -418,7 +419,7 @@ static std::unique_ptr parseLeaf ( if (!vBlob.second) throw std::invalid_argument ("invalid data"); - ret = std::make_unique (field, vBlob.first.data (), + ret = detail::make_stvar (field, vBlob.first.data (), vBlob.first.size ()); } catch (...) @@ -432,7 +433,7 @@ static std::unique_ptr parseLeaf ( case STI_AMOUNT: try { - ret = std::make_unique (amountFromJson (field, value)); + ret = detail::make_stvar (amountFromJson (field, value)); } catch (...) { @@ -451,15 +452,14 @@ static std::unique_ptr parseLeaf ( try { - std::unique_ptr tail = - std::make_unique (field); + STVector256 tail (field); for (Json::UInt i = 0; value.isValidIndex (i); ++i) { uint256 s; s.SetHex (value[i].asString ()); - tail->push_back (s); + tail.push_back (s); } - ret = std::move (tail); + ret = detail::make_stvar (std::move (tail)); } catch (...) { @@ -478,8 +478,7 @@ static std::unique_ptr parseLeaf ( try { - std::unique_ptr tail = - std::make_unique (field); + STPathSet tail (field); for (Json::UInt i = 0; value.isValidIndex (i); ++i) { @@ -597,9 +596,9 @@ static std::unique_ptr parseLeaf ( p.emplace_back (uAccount, uCurrency, uIssuer, hasCurrency); } - tail->push_back (p); + tail.push_back (p); } - ret = std::move (tail); + ret = detail::make_stvar (std::move (tail)); } catch (...) { @@ -625,7 +624,7 @@ static std::unique_ptr parseLeaf ( { Account account; account.SetHex (strValue); - ret = std::make_unique (field, account); + ret = detail::make_stvar (field, account); } else { @@ -639,7 +638,7 @@ static std::unique_ptr parseLeaf ( } ret = - std::make_unique (field, a.getAccountID ()); + detail::make_stvar (field, a.getAccountID ()); } } catch (...) @@ -661,42 +660,37 @@ static std::unique_ptr parseLeaf ( static const int maxDepth = 64; // Forward declaration since parseObject() and parseArray() call each other. -static bool parseArray ( +static boost::optional parseArray ( std::string const& json_name, Json::Value const& json, SField const& inName, int depth, - std::unique_ptr & sub_array, Json::Value& error); -static bool parseObject ( +static boost::optional parseObject ( std::string const& json_name, Json::Value const& json, SField const& inName, int depth, - std::unique_ptr & sub_object, Json::Value& error) { if (! json.isObject ()) { error = not_an_object (json_name); - return false; + return boost::none; } if (depth > maxDepth) { error = too_deep (json_name); - return false; + return boost::none; } - auto name (&inName); + STObject data (inName); - boost::ptr_vector data; - Json::Value::Members members (json.getMemberNames ()); - - for (auto const& fieldName : members) + for (auto const& fieldName : json.getMemberNames ()) { Json::Value const& value = json [fieldName]; @@ -705,7 +699,7 @@ static bool parseObject ( if (field == sfInvalid) { error = unknown_field (json_name, fieldName); - return false; + return boost::none; } switch (field.fieldType) @@ -719,22 +713,21 @@ static bool parseObject ( if (! value.isObject ()) { error = not_an_object (json_name, fieldName); - return false; + return boost::none; } try { - std::unique_ptr sub_object_; - bool const success (parseObject (json_name + "." + fieldName, - value, field, depth + 1, sub_object_, error)); - if (! success) - return false; - data.push_back (sub_object_.release ()); + auto ret = parseObject (json_name + "." + fieldName, + value, field, depth + 1, error); + if (! ret) + return boost::none; + data.emplace_back (std::move (*ret)); } catch (...) { error = invalid_data (json_name, fieldName); - return false; + return boost::none; } break; @@ -743,17 +736,16 @@ static bool parseObject ( case STI_ARRAY: try { - std::unique_ptr sub_array_; - bool const success (parseArray (json_name + "." + fieldName, - value, field, depth + 1, sub_array_, error)); - if (! success) - return false; - data.push_back (sub_array_.release ()); + auto array = parseArray (json_name + "." + fieldName, + value, field, depth + 1, error); + if (array == boost::none) + return boost::none; + data.emplace_back (std::move (*array)); } catch (...) { error = invalid_data (json_name, fieldName); - return false; + return boost::none; } break; @@ -761,46 +753,44 @@ static bool parseObject ( // Everything else (types that don't recurse). default: { - std::unique_ptr serTyp = - parseLeaf (json_name, fieldName, name, value, error); + auto leaf = + parseLeaf (json_name, fieldName, &inName, value, error); - if (!serTyp) - return false; + if (!leaf) + return boost::none; - data.push_back (serTyp.release ()); + data.emplace_back (std::move (*leaf)); } break; } } - sub_object = std::make_unique (*name, data); - return true; + return std::move (data); } -static bool parseArray ( +static boost::optional parseArray ( std::string const& json_name, Json::Value const& json, SField const& inName, int depth, - std::unique_ptr & sub_array, Json::Value& error) { if (! json.isArray ()) { error = not_an_array (json_name); - return false; + return boost::none; } if (depth > maxDepth) { error = too_deep (json_name); - return false; + return boost::none; } try { - std::unique_ptr tail = std::make_unique (inName); + STArray tail (inName); for (Json::UInt i = 0; json.isValidIndex (i); ++i) { @@ -810,7 +800,7 @@ static bool parseArray ( if (!isObject || !singleKey) { error = singleton_expected (json_name, i); - return false; + return boost::none; } // TODO: There doesn't seem to be a nice way to get just the @@ -822,35 +812,34 @@ static bool parseArray ( if (nameField == sfInvalid) { error = unknown_field (json_name, objectName); - return false; + return boost::none; } Json::Value const objectFields (json[i][objectName]); - std::unique_ptr sub_object_; - { - std::stringstream ss; - ss << json_name << "." << - "[" << i << "]." << objectName; - bool const success (parseObject (ss.str (), objectFields, - nameField, depth + 1, sub_object_, error)); + std::stringstream ss; + ss << json_name << "." << + "[" << i << "]." << objectName; - if (! success || - (sub_object_->getFName().fieldType != STI_OBJECT)) - { - return false; - } - } - tail->push_back (*sub_object_); + auto ret = parseObject (ss.str (), objectFields, + nameField, depth + 1, error); + + if (! ret || + (ret->getFName().fieldType != STI_OBJECT)) + { + return boost::none; + } + + tail.push_back (std::move (*ret)); } - sub_array = std::move (tail); + + return detail::make_stvar (std::move (tail)); } catch (...) { error = invalid_data (json_name); - return false; + return boost::none; } - return true; } } // STParsedJSONDetail @@ -862,7 +851,7 @@ STParsedJSONObject::STParsedJSONObject ( Json::Value const& json) { using namespace STParsedJSONDetail; - parseObject (name, json, sfGeneric, 0, object, error); + object = std::move (parseObject (name, json, sfGeneric, 0, error)); } //------------------------------------------------------------------------------ @@ -872,7 +861,17 @@ STParsedJSONArray::STParsedJSONArray ( Json::Value const& json) { using namespace STParsedJSONDetail; - parseArray (name, json, sfGeneric, 0, array, error); + auto arr = parseArray (name, json, sfGeneric, 0, error); + if (!arr) + array = boost::none; + else + { + auto p = dynamic_cast (&arr->get()); + if (p == nullptr) + array = boost::none; + else + array = std::move (*p); + } } diff --git a/src/ripple/protocol/impl/STTx.cpp b/src/ripple/protocol/impl/STTx.cpp index a4002b88f..b9a2d3141 100644 --- a/src/ripple/protocol/impl/STTx.cpp +++ b/src/ripple/protocol/impl/STTx.cpp @@ -25,7 +25,6 @@ #include #include #include -#include #include #include #include @@ -54,8 +53,8 @@ STTx::STTx (TxType type) setFieldU16 (sfTransactionType, format->getType ()); } -STTx::STTx (STObject const& object) - : STObject (object) +STTx::STTx (STObject&& object) + : STObject (std::move (object)) , sig_state_ (boost::indeterminate) { tx_type_ = static_cast (getFieldU16 (sfTransactionType)); diff --git a/src/ripple/protocol/impl/STVar.h b/src/ripple/protocol/impl/STVar.h index 599c7f69b..6191b2620 100644 --- a/src/ripple/protocol/impl/STVar.h +++ b/src/ripple/protocol/impl/STVar.h @@ -88,7 +88,14 @@ public: STBase const& operator*() const { return get(); } STBase const* operator->() const { return &get(); } + template + friend + STVar + make_stvar(Args&&... args); + private: + STVar() = default; + STVar (SerializedTypeID id, SField const& name); void destroy(); @@ -113,6 +120,16 @@ private: } }; +template +inline +STVar +make_stvar(Args&&... args) +{ + STVar st; + st.construct(std::forward(args)...); + return std::move(st); +} + inline bool operator== (STVar const& lhs, STVar const& rhs) diff --git a/src/ripple/protocol/tests/STObject.test.cpp b/src/ripple/protocol/tests/STObject.test.cpp index 52d768ca0..0432384c0 100644 --- a/src/ripple/protocol/tests/STObject.test.cpp +++ b/src/ripple/protocol/tests/STObject.test.cpp @@ -75,7 +75,7 @@ public: bool parsedOK (parseJSONString(faulty, faultyJson)); unexpected(!parsedOK, "failed to parse"); STParsedJSONObject parsed ("test", faultyJson); - expect (parsed.object.get() == nullptr, + expect (! parsed.object, "It should have thrown. " "Immediate children of STArray encoded as json must " "have one key only."); diff --git a/src/ripple/protocol/tests/STTx.test.cpp b/src/ripple/protocol/tests/STTx.test.cpp index de51b1d29..4386abfd7 100644 --- a/src/ripple/protocol/tests/STTx.test.cpp +++ b/src/ripple/protocol/tests/STTx.test.cpp @@ -61,15 +61,13 @@ public: } STParsedJSONObject parsed ("test", j.getJson (0)); - std::unique_ptr new_obj (std::move (parsed.object)); - - if (new_obj.get () == nullptr) + if (!parsed.object) fail ("Unable to build object from json"); - if (STObject (j) != *new_obj) + if (STObject (j) != parsed.object) { log << "ORIG: " << j.getJson (0); - log << "BUILT " << new_obj->getJson (0); + log << "BUILT " << parsed.object->getJson (0); fail ("Built a different transaction"); } else diff --git a/src/ripple/rpc/handlers/RipplePathFind.cpp b/src/ripple/rpc/handlers/RipplePathFind.cpp index 9b255d330..7c526f489 100644 --- a/src/ripple/rpc/handlers/RipplePathFind.cpp +++ b/src/ripple/rpc/handlers/RipplePathFind.cpp @@ -249,11 +249,11 @@ ripplePathFind(RippleLineCache::pointer const& cache, Json::Value pathSet = Json::objectValue; pathSet[jss::Paths] = contextPaths.get(); STParsedJSONObject paths("pathSet", pathSet); - if (paths.object.get() == nullptr) + if (! paths.object) return std::make_pair(false, paths.error); else { - spsComputed = paths.object.get()->getFieldPathSet(sfPaths); + spsComputed = paths.object->getFieldPathSet(sfPaths); WriteLog(lsTRACE, RPCHandler) << "ripple_path_find: Paths: " << spsComputed.getJson(0); } diff --git a/src/ripple/rpc/impl/TransactionSign.cpp b/src/ripple/rpc/impl/TransactionSign.cpp index 02f1c3be0..d720548ad 100644 --- a/src/ripple/rpc/impl/TransactionSign.cpp +++ b/src/ripple/rpc/impl/TransactionSign.cpp @@ -445,15 +445,14 @@ transactionSign ( } STParsedJSONObject parsed (std::string (jss::tx_json), tx_json); - if (!parsed.object.get()) + if (! parsed.object) { jvResult [jss::error] = parsed.error [jss::error]; jvResult [jss::error_code] = parsed.error [jss::error_code]; jvResult [jss::error_message] = parsed.error [jss::error_message]; return jvResult; } - std::unique_ptr sopTrans = std::move(parsed.object); - sopTrans->setFieldVL ( + parsed.object->setFieldVL ( sfSigningPubKey, keypair.publicKey.getAccountPublic()); @@ -461,7 +460,7 @@ transactionSign ( try { - stpTrans = std::make_shared (*sopTrans); + stpTrans = std::make_shared (std::move (*parsed.object)); } catch (std::exception&) {