From 6f93e1003e4780ee2a7285f8382ba4d9059b0abf Mon Sep 17 00:00:00 2001 From: Alex Kremer Date: Mon, 15 May 2023 17:57:21 +0100 Subject: [PATCH] Address cppcheck issues (#640) Fixes #639 --- src/backend/cassandra/impl/Cluster.cpp | 2 +- src/backend/cassandra/impl/Future.cpp | 6 +- src/log/Logger.h | 20 +-- src/rpc/RPCHelpers.cpp | 182 ------------------------- src/rpc/RPCHelpers.h | 12 -- src/rpc/handlers/AccountInfo.cpp | 8 +- src/rpc/handlers/AccountObjects.cpp | 7 +- 7 files changed, 25 insertions(+), 212 deletions(-) diff --git a/src/backend/cassandra/impl/Cluster.cpp b/src/backend/cassandra/impl/Cluster.cpp index 34426dce8..823d6e6e5 100644 --- a/src/backend/cassandra/impl/Cluster.cpp +++ b/src/backend/cassandra/impl/Cluster.cpp @@ -102,7 +102,7 @@ void Cluster::setupContactPoints(Settings::ContactPoints const& points) { using std::to_string; - auto throwErrorIfNeeded = [](CassError rc, std::string const label, std::string const value) { + auto throwErrorIfNeeded = [](CassError rc, std::string const& label, std::string const& value) { if (rc != CASS_OK) throw std::runtime_error("Cassandra: Error setting " + label + " [" + value + "]: " + cass_error_desc(rc)); }; diff --git a/src/backend/cassandra/impl/Future.cpp b/src/backend/cassandra/impl/Future.cpp index 6b6965bf4..05f5122ea 100644 --- a/src/backend/cassandra/impl/Future.cpp +++ b/src/backend/cassandra/impl/Future.cpp @@ -39,7 +39,7 @@ Future::await() const { if (auto const rc = cass_future_error_code(*this); rc) { - auto errMsg = [this](std::string label) { + auto errMsg = [this](std::string const& label) { char const* message; std::size_t len; cass_future_error_message(*this, &message, &len); @@ -55,7 +55,7 @@ Future::get() const { if (auto const rc = cass_future_error_code(*this); rc) { - auto const errMsg = [this](std::string label) { + auto const errMsg = [this](std::string const& label) { char const* message; std::size_t len; cass_future_error_message(*this, &message, &len); @@ -76,7 +76,7 @@ invokeHelper(CassFuture* ptr, void* cbPtr) auto* cb = static_cast(cbPtr); if (auto const rc = cass_future_error_code(ptr); rc) { - auto const errMsg = [&ptr](std::string label) { + auto const errMsg = [&ptr](std::string const& label) { char const* message; std::size_t len; cass_future_error_message(ptr, &message, &len); diff --git a/src/log/Logger.h b/src/log/Logger.h index 9bc4b6e68..96f52e627 100644 --- a/src/log/Logger.h +++ b/src/log/Logger.h @@ -111,16 +111,6 @@ BOOST_LOG_ATTRIBUTE_KEYWORD(log_channel, "Channel", std::string); std::ostream& operator<<(std::ostream& stream, Severity sev); -/** - * @brief Custom JSON parser for @ref Severity. - * - * @param value The JSON string to parse - * @return Severity The parsed severity - * @throws std::runtime_error Thrown if severity is not in the right format - */ -Severity -tag_invoke(boost::json::value_to_tag, boost::json::value const& value); - /** * @brief A simple thread-safe logger for the channel specified * in the constructor. @@ -185,6 +175,16 @@ class Logger final private: [[nodiscard]] std::string pretty_path(source_location_t const& loc, size_t max_depth = 3) const; + + /** + * @brief Custom JSON parser for @ref Severity. + * + * @param value The JSON string to parse + * @return Severity The parsed severity + * @throws std::runtime_error Thrown if severity is not in the right format + */ + friend Severity + tag_invoke(boost::json::value_to_tag, boost::json::value const& value); }; public: diff --git a/src/rpc/RPCHelpers.cpp b/src/rpc/RPCHelpers.cpp index 00cbc821e..999874f51 100644 --- a/src/rpc/RPCHelpers.cpp +++ b/src/rpc/RPCHelpers.cpp @@ -1459,186 +1459,4 @@ getNFTID(boost::json::object const& request) return tokenid; } -// TODO - this function is long and shouldn't be responsible for as much as it -// is. Split it out into some helper functions. -std::variant -traverseTransactions( - std::shared_ptr const& backend, - Web::Context const& context, - std::function const& backend, - bool const, - std::optional const&, - boost::asio::yield_context& yield)> transactionFetcher) -{ - auto request = context.params; - boost::json::object response = {}; - - bool const binary = getBool(request, JS(binary), false); - bool const forward = getBool(request, JS(forward), false); - - std::optional cursor; - - if (request.contains(JS(marker))) - { - if (!request.at(JS(marker)).is_object()) - return Status{RippledError::rpcINVALID_PARAMS, "invalidMarker"}; - auto const& obj = request.at(JS(marker)).as_object(); - - std::optional transactionIndex = {}; - if (obj.contains(JS(seq))) - { - if (!obj.at(JS(seq)).is_int64()) - return Status{RippledError::rpcINVALID_PARAMS, "transactionIndexNotInt"}; - - transactionIndex = boost::json::value_to(obj.at(JS(seq))); - } - - std::optional ledgerIndex = {}; - if (obj.contains(JS(ledger))) - { - if (!obj.at(JS(ledger)).is_int64()) - return Status{RippledError::rpcINVALID_PARAMS, "ledgerIndexNotInt"}; - - ledgerIndex = boost::json::value_to(obj.at(JS(ledger))); - } - - if (!transactionIndex || !ledgerIndex) - return Status{RippledError::rpcINVALID_PARAMS, "missingLedgerOrSeq"}; - - cursor = {*ledgerIndex, *transactionIndex}; - } - - auto minIndex = context.range.minSequence; - auto maxIndex = context.range.maxSequence; - std::optional min; - std::optional max; - - if (request.contains(JS(ledger_index_min))) - { - if (!request.at(JS(ledger_index_min)).is_int64()) - { - return Status{RippledError::rpcINVALID_PARAMS, "ledgerSeqMinNotNumber"}; - } - - min = request.at(JS(ledger_index_min)).as_int64(); - - if (*min != -1) - { - if (context.range.maxSequence < *min || context.range.minSequence > *min) - return Status{RippledError::rpcLGR_IDX_MALFORMED, "ledgerSeqMinOutOfRange"}; - else - minIndex = static_cast(*min); - } - - if (forward && !cursor) - cursor = {minIndex, 0}; - } - - if (request.contains(JS(ledger_index_max))) - { - if (!request.at(JS(ledger_index_max)).is_int64()) - { - return Status{RippledError::rpcINVALID_PARAMS, "ledgerSeqMaxNotNumber"}; - } - - max = request.at(JS(ledger_index_max)).as_int64(); - - if (*max != -1) - { - if (context.range.maxSequence < *max || context.range.minSequence > *max) - return Status{RippledError::rpcLGR_IDXS_INVALID}; - else - maxIndex = static_cast(*max); - } - - if (minIndex > maxIndex) - return Status{RippledError::rpcINVALID_PARAMS, "invalidIndex"}; - - if (!forward && !cursor) - cursor = {maxIndex, INT32_MAX}; - } - - if (max && min && *max < *min) - { - return Status{RippledError::rpcLGR_IDXS_INVALID, "lgrIdxsInvalid"}; - } - - if (request.contains(JS(ledger_index)) || request.contains(JS(ledger_hash))) - { - if (request.contains(JS(ledger_index_max)) || request.contains(JS(ledger_index_min))) - return Status{RippledError::rpcINVALID_PARAMS, "containsLedgerSpecifierAndRange"}; - - auto v = ledgerInfoFromRequest(backend, context); - if (auto status = std::get_if(&v); status) - return *status; - - maxIndex = minIndex = std::get(v).seq; - } - - if (!cursor) - { - if (forward) - cursor = {minIndex, 0}; - else - cursor = {maxIndex, INT32_MAX}; - } - - boost::json::array txns; - auto [blobs, retCursor] = transactionFetcher(backend, forward, cursor, context.yield); - auto timeDiff = util::timed([&, &retCursor = retCursor, &blobs = blobs]() { - if (retCursor) - { - boost::json::object cursorJson; - cursorJson[JS(ledger)] = retCursor->ledgerSequence; - cursorJson[JS(seq)] = retCursor->transactionIndex; - response[JS(marker)] = cursorJson; - } - - for (auto const& txnPlusMeta : blobs) - { - if ((txnPlusMeta.ledgerSequence < minIndex && !forward) || - (txnPlusMeta.ledgerSequence > maxIndex && forward)) - { - response.erase(JS(marker)); - break; - } - else if (txnPlusMeta.ledgerSequence > maxIndex && !forward) - { - gLog.debug() << "Skipping over transactions from incomplete ledger"; - continue; - } - - boost::json::object obj; - - if (!binary) - { - auto [txn, meta] = toExpandedJson(txnPlusMeta); - obj[JS(meta)] = meta; - obj[JS(tx)] = txn; - obj[JS(tx)].as_object()[JS(ledger_index)] = txnPlusMeta.ledgerSequence; - obj[JS(tx)].as_object()[JS(date)] = txnPlusMeta.date; - } - else - { - obj[JS(meta)] = ripple::strHex(txnPlusMeta.metadata); - obj[JS(tx_blob)] = ripple::strHex(txnPlusMeta.transaction); - obj[JS(ledger_index)] = txnPlusMeta.ledgerSequence; - obj[JS(date)] = txnPlusMeta.date; - } - obj[JS(validated)] = true; - txns.push_back(obj); - } - - response[JS(ledger_index_min)] = minIndex; - response[JS(ledger_index_max)] = maxIndex; - response[JS(transactions)] = txns; - }); - gLog.info() << "serialization took " << timeDiff - - << " milliseconds"; - - return response; -} - } // namespace RPC diff --git a/src/rpc/RPCHelpers.h b/src/rpc/RPCHelpers.h index b3a3dbc9d..1dfb6dbfe 100644 --- a/src/rpc/RPCHelpers.h +++ b/src/rpc/RPCHelpers.h @@ -270,18 +270,6 @@ specifiesCurrentOrClosedLedger(boost::json::object const& request); std::variant getNFTID(boost::json::object const& request); -// This function is the driver for both `account_tx` and `nft_tx` and should -// be used for any future transaction enumeration APIs. -std::variant -traverseTransactions( - std::shared_ptr const& backend, - Web::Context const& context, - std::function const& backend, - bool const, - std::optional const&, - boost::asio::yield_context& yield)> transactionFetcher); - template void logDuration(Web::Context const& ctx, T const& dur) diff --git a/src/rpc/handlers/AccountInfo.cpp b/src/rpc/handlers/AccountInfo.cpp index cedcf6af4..7586dd043 100644 --- a/src/rpc/handlers/AccountInfo.cpp +++ b/src/rpc/handlers/AccountInfo.cpp @@ -92,8 +92,12 @@ tag_invoke(boost::json::value_from_tag, boost::json::value& jv, AccountInfoHandl if (output.signerLists) { auto signers = boost::json::array(); - for (auto const& signerList : output.signerLists.value()) - signers.push_back(toJson(signerList)); + std::transform( + std::cbegin(output.signerLists.value()), + std::cend(output.signerLists.value()), + std::back_inserter(signers), + [](auto const& signerList) { return toJson(signerList); }); + jv.as_object()[JS(account_data)].as_object()[JS(signer_lists)] = std::move(signers); } } diff --git a/src/rpc/handlers/AccountObjects.cpp b/src/rpc/handlers/AccountObjects.cpp index da1316af0..c5fcca6d7 100644 --- a/src/rpc/handlers/AccountObjects.cpp +++ b/src/rpc/handlers/AccountObjects.cpp @@ -83,8 +83,11 @@ void tag_invoke(boost::json::value_from_tag, boost::json::value& jv, AccountObjectsHandler::Output const& output) { auto objects = boost::json::array{}; - for (auto const& sle : output.accountObjects) - objects.push_back(toJson(sle)); + std::transform( + std::cbegin(output.accountObjects), + std::cend(output.accountObjects), + std::back_inserter(objects), + [](auto const& sle) { return toJson(sle); }); jv = { {JS(ledger_hash), output.ledgerHash},