From cd27b5f2bd839bb0cd31696e74c5a42c5b06466f Mon Sep 17 00:00:00 2001 From: seelabs Date: Mon, 7 Jun 2021 11:26:27 -0400 Subject: [PATCH] Some code cleanups tagged by static analysis --- src/ripple/app/ledger/Ledger.cpp | 2 +- src/ripple/app/ledger/OrderBookDB.h | 2 +- src/ripple/app/main/NodeStoreScheduler.h | 2 +- src/ripple/app/misc/Manifest.h | 2 +- src/ripple/app/misc/SHAMapStoreImp.cpp | 2 +- src/ripple/app/misc/SHAMapStoreImp.h | 2 +- src/ripple/app/misc/impl/Manifest.cpp | 2 +- src/ripple/app/rdb/RelationalDBInterface.h | 8 ++++---- .../app/rdb/RelationalDBInterface_global.h | 4 ++-- .../backend/RelationalDBInterfaceSqlite.cpp | 18 +++++++++--------- .../rdb/backend/RelationalDBInterfaceSqlite.h | 6 +++--- .../rdb/impl/RelationalDBInterface_global.cpp | 4 ++-- src/ripple/app/reporting/ReportingETL.h | 2 +- src/ripple/core/SociDB.h | 6 +++--- src/ripple/core/impl/SociDB.cpp | 4 ++-- src/ripple/ledger/TxMeta.h | 2 +- src/ripple/net/DatabaseBody.h | 4 ++-- src/ripple/net/impl/DatabaseBody.ipp | 4 ++-- src/ripple/nodestore/impl/DatabaseShardImp.cpp | 3 ++- src/ripple/nodestore/impl/Shard.cpp | 6 +++--- src/ripple/nodestore/impl/Shard.h | 5 +++-- src/ripple/protocol/Quality.h | 2 +- src/ripple/rpc/ShardArchiveHandler.h | 4 ++-- src/ripple/rpc/handlers/GetCounts.cpp | 6 +++--- src/ripple/rpc/handlers/TxHistory.cpp | 2 +- src/ripple/rpc/impl/Handler.cpp | 3 ++- src/test/basics/PerfLog_test.cpp | 2 +- src/test/nodestore/DatabaseShard_test.cpp | 8 ++++---- 28 files changed, 60 insertions(+), 57 deletions(-) diff --git a/src/ripple/app/ledger/Ledger.cpp b/src/ripple/app/ledger/Ledger.cpp index 5ef4851ba..0757bb4f4 100644 --- a/src/ripple/app/ledger/Ledger.cpp +++ b/src/ripple/app/ledger/Ledger.cpp @@ -141,7 +141,7 @@ public: txs_iter_impl(txs_iter_impl const&) = default; txs_iter_impl(bool metadata, SHAMap::const_iterator iter) - : metadata_(metadata), iter_(iter) + : metadata_(metadata), iter_(std::move(iter)) { } diff --git a/src/ripple/app/ledger/OrderBookDB.h b/src/ripple/app/ledger/OrderBookDB.h index a1983fdb4..3b6939013 100644 --- a/src/ripple/app/ledger/OrderBookDB.h +++ b/src/ripple/app/ledger/OrderBookDB.h @@ -31,7 +31,7 @@ namespace ripple { class OrderBookDB { public: - OrderBookDB(Application& app); + explicit OrderBookDB(Application& app); void setup(std::shared_ptr const& ledger); diff --git a/src/ripple/app/main/NodeStoreScheduler.h b/src/ripple/app/main/NodeStoreScheduler.h index 533b7497b..5c68dc24f 100644 --- a/src/ripple/app/main/NodeStoreScheduler.h +++ b/src/ripple/app/main/NodeStoreScheduler.h @@ -30,7 +30,7 @@ namespace ripple { class NodeStoreScheduler : public NodeStore::Scheduler { public: - NodeStoreScheduler(JobQueue& jobQueue); + explicit NodeStoreScheduler(JobQueue& jobQueue); void scheduleTask(NodeStore::Task& task) override; diff --git a/src/ripple/app/misc/Manifest.h b/src/ripple/app/misc/Manifest.h index 254d1758a..5f2b9619f 100644 --- a/src/ripple/app/misc/Manifest.h +++ b/src/ripple/app/misc/Manifest.h @@ -374,7 +374,7 @@ public: save( DatabaseCon& dbCon, std::string const& dbTable, - std::function isTrusted); + std::function const& isTrusted); /** Invokes the callback once for every populated manifest. diff --git a/src/ripple/app/misc/SHAMapStoreImp.cpp b/src/ripple/app/misc/SHAMapStoreImp.cpp index e312d507a..5fce4cd72 100644 --- a/src/ripple/app/misc/SHAMapStoreImp.cpp +++ b/src/ripple/app/misc/SHAMapStoreImp.cpp @@ -535,7 +535,7 @@ SHAMapStoreImp::makeBackendRotating(std::string path) void SHAMapStoreImp::clearSql( LedgerIndex lastRotated, - const std::string TableName, + std::string const& TableName, std::function()> const& getMinSeq, std::function const& deleteBeforeSeq) { diff --git a/src/ripple/app/misc/SHAMapStoreImp.h b/src/ripple/app/misc/SHAMapStoreImp.h index 6dc9c718c..7119bd3af 100644 --- a/src/ripple/app/misc/SHAMapStoreImp.h +++ b/src/ripple/app/misc/SHAMapStoreImp.h @@ -212,7 +212,7 @@ private: void clearSql( LedgerIndex lastRotated, - const std::string TableName, + std::string const& TableName, std::function()> const& getMinSeq, std::function const& deleteBeforeSeq); void diff --git a/src/ripple/app/misc/impl/Manifest.cpp b/src/ripple/app/misc/impl/Manifest.cpp index 4876701d8..2b9725030 100644 --- a/src/ripple/app/misc/impl/Manifest.cpp +++ b/src/ripple/app/misc/impl/Manifest.cpp @@ -542,7 +542,7 @@ void ManifestCache::save( DatabaseCon& dbCon, std::string const& dbTable, - std::function isTrusted) + std::function const& isTrusted) { std::lock_guard lock{apply_mutex_}; auto db = dbCon.checkoutDb(); diff --git a/src/ripple/app/rdb/RelationalDBInterface.h b/src/ripple/app/rdb/RelationalDBInterface.h index f6b29a473..a0d01545b 100644 --- a/src/ripple/app/rdb/RelationalDBInterface.h +++ b/src/ripple/app/rdb/RelationalDBInterface.h @@ -122,14 +122,14 @@ public: uint256 nodestoreHash; AccountTransactionsData( - TxMeta& meta, - uint256&& nodestoreHash, - beast::Journal& j) + TxMeta const& meta, + uint256 const& nodestoreHash, + beast::Journal j) : accounts(meta.getAffectedAccounts(j)) , ledgerSequence(meta.getLgrSeq()) , transactionIndex(meta.getIndex()) , txHash(meta.getTxID()) - , nodestoreHash(std::move(nodestoreHash)) + , nodestoreHash(nodestoreHash) { } }; diff --git a/src/ripple/app/rdb/RelationalDBInterface_global.h b/src/ripple/app/rdb/RelationalDBInterface_global.h index 8f62a47a0..aa50b2e97 100644 --- a/src/ripple/app/rdb/RelationalDBInterface_global.h +++ b/src/ripple/app/rdb/RelationalDBInterface_global.h @@ -75,7 +75,7 @@ void saveManifests( soci::session& session, std::string const& dbTable, - std::function isTrusted, + std::function const& isTrusted, hash_map const& map, beast::Journal j); @@ -237,7 +237,7 @@ setLastRotated(soci::session& session, LedgerIndex seq); std::pair, std::optional> openDatabaseBodyDb( DatabaseCon::Setup const& setup, - boost::filesystem::path path); + boost::filesystem::path const& path); /** * @brief databaseBodyDoPut Saves new fragment of downloaded file. diff --git a/src/ripple/app/rdb/backend/RelationalDBInterfaceSqlite.cpp b/src/ripple/app/rdb/backend/RelationalDBInterfaceSqlite.cpp index bece17cbe..b223e25ab 100644 --- a/src/ripple/app/rdb/backend/RelationalDBInterfaceSqlite.cpp +++ b/src/ripple/app/rdb/backend/RelationalDBInterfaceSqlite.cpp @@ -174,13 +174,13 @@ public: bool transactionDbHasSpace(Config const& config) override; - int + std::uint32_t getKBUsedAll() override; - int + std::uint32_t getKBUsedLedger() override; - int + std::uint32_t getKBUsedTransaction() override; void @@ -1473,7 +1473,7 @@ RelationalDBInterfaceSqliteImp::transactionDbHasSpace(Config const& config) }); } -int +std::uint32_t RelationalDBInterfaceSqliteImp::getKBUsedAll() { /* if database exists, use it */ @@ -1483,7 +1483,7 @@ RelationalDBInterfaceSqliteImp::getKBUsedAll() } /* else use shard databases */ - int sum = 0; + std::uint32_t sum = 0; iterateLedgerBack( {}, [&](soci::session& session, std::uint32_t shardIndex) { sum += ripple::getKBUsedAll(session); @@ -1492,7 +1492,7 @@ RelationalDBInterfaceSqliteImp::getKBUsedAll() return sum; } -int +std::uint32_t RelationalDBInterfaceSqliteImp::getKBUsedLedger() { /* if database exists, use it */ @@ -1502,7 +1502,7 @@ RelationalDBInterfaceSqliteImp::getKBUsedLedger() } /* else use shard databases */ - int sum = 0; + std::uint32_t sum = 0; iterateLedgerBack( {}, [&](soci::session& session, std::uint32_t shardIndex) { sum += ripple::getKBUsedDB(session); @@ -1511,7 +1511,7 @@ RelationalDBInterfaceSqliteImp::getKBUsedLedger() return sum; } -int +std::uint32_t RelationalDBInterfaceSqliteImp::getKBUsedTransaction() { /* if database exists, use it */ @@ -1521,7 +1521,7 @@ RelationalDBInterfaceSqliteImp::getKBUsedTransaction() } /* else use shard databases */ - int sum = 0; + std::uint32_t sum = 0; iterateTransactionBack( {}, [&](soci::session& session, std::uint32_t shardIndex) { sum += ripple::getKBUsedDB(session); diff --git a/src/ripple/app/rdb/backend/RelationalDBInterfaceSqlite.h b/src/ripple/app/rdb/backend/RelationalDBInterfaceSqlite.h index 65b52f0e0..085f59628 100644 --- a/src/ripple/app/rdb/backend/RelationalDBInterfaceSqlite.h +++ b/src/ripple/app/rdb/backend/RelationalDBInterfaceSqlite.h @@ -266,14 +266,14 @@ public: * @brief getKBUsedAll Returns space used by all databases. * @return Space in kilobytes. */ - virtual int + virtual uint32_t getKBUsedAll() = 0; /** * @brief getKBUsedLedger Returns space used by ledger database. * @return Space in kilobytes. */ - virtual int + virtual uint32_t getKBUsedLedger() = 0; /** @@ -281,7 +281,7 @@ public: * database. * @return Space in kilobytes. */ - virtual int + virtual uint32_t getKBUsedTransaction() = 0; /** diff --git a/src/ripple/app/rdb/impl/RelationalDBInterface_global.cpp b/src/ripple/app/rdb/impl/RelationalDBInterface_global.cpp index af98cac0f..5c000fc9a 100644 --- a/src/ripple/app/rdb/impl/RelationalDBInterface_global.cpp +++ b/src/ripple/app/rdb/impl/RelationalDBInterface_global.cpp @@ -104,7 +104,7 @@ void saveManifests( soci::session& session, std::string const& dbTable, - std::function isTrusted, + std::function const& isTrusted, hash_map const& map, beast::Journal j) { @@ -410,7 +410,7 @@ setLastRotated(soci::session& session, LedgerIndex seq) std::pair, std::optional> openDatabaseBodyDb( DatabaseCon::Setup const& setup, - boost::filesystem::path path) + boost::filesystem::path const& path) { // SOCI requires boost::optional (not std::optional) as the parameter. boost::optional pathFromDb; diff --git a/src/ripple/app/reporting/ReportingETL.h b/src/ripple/app/reporting/ReportingETL.h index aa795fb6d..540cc5bfd 100644 --- a/src/ripple/app/reporting/ReportingETL.h +++ b/src/ripple/app/reporting/ReportingETL.h @@ -266,7 +266,7 @@ private: ThreadSafeQueue>& writeQueue); public: - ReportingETL(Application& app); + explicit ReportingETL(Application& app); ~ReportingETL() { diff --git a/src/ripple/core/SociDB.h b/src/ripple/core/SociDB.h index 1a9ada1d6..621754098 100644 --- a/src/ripple/core/SociDB.h +++ b/src/ripple/core/SociDB.h @@ -57,7 +57,7 @@ class BasicConfig; class DBConfig { std::string connectionString_; - DBConfig(std::string const& dbPath); + explicit DBConfig(std::string const& dbPath); public: DBConfig(BasicConfig const& config, std::string const& dbName); @@ -97,9 +97,9 @@ open( std::string const& beName, std::string const& connectionString); -size_t +std::uint32_t getKBUsedAll(soci::session& s); -size_t +std::uint32_t getKBUsedDB(soci::session& s); void diff --git a/src/ripple/core/impl/SociDB.cpp b/src/ripple/core/impl/SociDB.cpp index 4128c8138..419784c12 100644 --- a/src/ripple/core/impl/SociDB.cpp +++ b/src/ripple/core/impl/SociDB.cpp @@ -126,7 +126,7 @@ getConnection(soci::session& s) return result; } -size_t +std::uint32_t getKBUsedAll(soci::session& s) { if (!getConnection(s)) @@ -135,7 +135,7 @@ getKBUsedAll(soci::session& s) sqlite_api::sqlite3_memory_used() / kilobytes(1)); } -size_t +std::uint32_t getKBUsedDB(soci::session& s) { // This function will have to be customized when other backends are added diff --git a/src/ripple/ledger/TxMeta.h b/src/ripple/ledger/TxMeta.h index 3b35abf55..1335e71e0 100644 --- a/src/ripple/ledger/TxMeta.h +++ b/src/ripple/ledger/TxMeta.h @@ -50,7 +50,7 @@ public: TxMeta(uint256 const& txID, std::uint32_t ledger, STObject const&); uint256 const& - getTxID() + getTxID() const { return mTransactionID; } diff --git a/src/ripple/net/DatabaseBody.h b/src/ripple/net/DatabaseBody.h index c884dddd8..c828e5bf1 100644 --- a/src/ripple/net/DatabaseBody.h +++ b/src/ripple/net/DatabaseBody.h @@ -105,7 +105,7 @@ public: */ void open( - boost::filesystem::path path, + boost::filesystem::path const& path, Config const& config, boost::asio::io_service& io_service, boost::system::error_code& ec); @@ -159,7 +159,7 @@ public: put(ConstBufferSequence const& buffers, boost::system::error_code& ec); void - do_put(std::string data); + do_put(std::string const& data); // This function is called when writing is complete. // It is an opportunity to perform any final actions diff --git a/src/ripple/net/impl/DatabaseBody.ipp b/src/ripple/net/impl/DatabaseBody.ipp index e072d998e..061a63025 100644 --- a/src/ripple/net/impl/DatabaseBody.ipp +++ b/src/ripple/net/impl/DatabaseBody.ipp @@ -43,7 +43,7 @@ DatabaseBody::value_type::close() inline void DatabaseBody::value_type::open( - boost::filesystem::path path, + boost::filesystem::path const& path, Config const& config, boost::asio::io_service& io_service, boost::system::error_code& ec) @@ -156,7 +156,7 @@ DatabaseBody::reader::put( } inline void -DatabaseBody::reader::do_put(std::string data) +DatabaseBody::reader::do_put(std::string const& data) { using namespace boost::asio; diff --git a/src/ripple/nodestore/impl/DatabaseShardImp.cpp b/src/ripple/nodestore/impl/DatabaseShardImp.cpp index 50cb1c012..eb5bab141 100644 --- a/src/ripple/nodestore/impl/DatabaseShardImp.cpp +++ b/src/ripple/nodestore/impl/DatabaseShardImp.cpp @@ -970,7 +970,8 @@ DatabaseShardImp::doImportDatabase() if (isStopping()) return; - auto const ledger{loadByIndex(*ledgerSeq, app_, false)}; + // Not const so it may be moved later + auto ledger{loadByIndex(*ledgerSeq, app_, false)}; if (!ledger || ledger->info().seq != ledgerSeq) break; diff --git a/src/ripple/nodestore/impl/Shard.cpp b/src/ripple/nodestore/impl/Shard.cpp index 4be75c458..dc5033c96 100644 --- a/src/ripple/nodestore/impl/Shard.cpp +++ b/src/ripple/nodestore/impl/Shard.cpp @@ -1218,18 +1218,18 @@ Shard::Count Shard::makeBackendCount() { if (stop_ || busy_) - return {nullptr}; + return Shard::Count{nullptr}; std::lock_guard lock(mutex_); if (!backend_) { JLOG(j_.error()) << "shard " << index_ << " not initialized"; - return {nullptr}; + return Shard::Count{nullptr}; } if (!backend_->isOpen()) { if (!open(lock)) - return {nullptr}; + return Shard::Count{nullptr}; } else if (state_ == ShardState::finalized) lastAccess_ = std::chrono::steady_clock::now(); diff --git a/src/ripple/nodestore/impl/Shard.h b/src/ripple/nodestore/impl/Shard.h index 0f78f82c1..4fc17a8d4 100644 --- a/src/ripple/nodestore/impl/Shard.h +++ b/src/ripple/nodestore/impl/Shard.h @@ -262,7 +262,8 @@ private: other.counter_ = nullptr; } - Count(std::atomic* counter) noexcept : counter_(counter) + explicit Count(std::atomic* counter) noexcept + : counter_(counter) { if (counter_) ++(*counter_); @@ -274,7 +275,7 @@ private: --(*counter_); } - operator bool() const noexcept + explicit operator bool() const noexcept { return counter_ != nullptr; } diff --git a/src/ripple/protocol/Quality.h b/src/ripple/protocol/Quality.h index 8626fe0ce..9de137d87 100644 --- a/src/ripple/protocol/Quality.h +++ b/src/ripple/protocol/Quality.h @@ -134,7 +134,7 @@ public: /** Create a quality from the ratio of two amounts. */ template - Quality(TAmounts const& amount) + explicit Quality(TAmounts const& amount) : Quality(Amounts(toSTAmount(amount.in), toSTAmount(amount.out))) { } diff --git a/src/ripple/rpc/ShardArchiveHandler.h b/src/ripple/rpc/ShardArchiveHandler.h index 221624885..b5a288968 100644 --- a/src/ripple/rpc/ShardArchiveHandler.h +++ b/src/ripple/rpc/ShardArchiveHandler.h @@ -56,7 +56,7 @@ public: static std::unique_ptr tryMakeRecoveryHandler(Application& app); - ShardArchiveHandler(Application& app); + explicit ShardArchiveHandler(Application& app); virtual ~ShardArchiveHandler() = default; @@ -163,7 +163,7 @@ private: class RecoveryHandler : public ShardArchiveHandler { public: - RecoveryHandler(Application& app); + explicit RecoveryHandler(Application& app); }; } // namespace RPC diff --git a/src/ripple/rpc/handlers/GetCounts.cpp b/src/ripple/rpc/handlers/GetCounts.cpp index fd189ebfe..d59e7014b 100644 --- a/src/ripple/rpc/handlers/GetCounts.cpp +++ b/src/ripple/rpc/handlers/GetCounts.cpp @@ -75,9 +75,9 @@ getCountsJson(Application& app, int minObjectCount) if (!app.config().reporting() && app.config().useTxTables()) { - int dbKB = dynamic_cast( - &app.getRelationalDBInterface()) - ->getKBUsedAll(); + auto dbKB = dynamic_cast( + &app.getRelationalDBInterface()) + ->getKBUsedAll(); if (dbKB > 0) ret[jss::dbKBTotal] = dbKB; diff --git a/src/ripple/rpc/handlers/TxHistory.cpp b/src/ripple/rpc/handlers/TxHistory.cpp index 266327e91..7fa7fc76f 100644 --- a/src/ripple/rpc/handlers/TxHistory.cpp +++ b/src/ripple/rpc/handlers/TxHistory.cpp @@ -63,7 +63,7 @@ doTxHistory(RPC::JsonContext& context) if (context.app.config().reporting()) obj["used_postgres"] = true; - for (auto t : trans) + for (auto const& t : trans) txs.append(t->getJson(JsonOptions::none)); return obj; diff --git a/src/ripple/rpc/impl/Handler.cpp b/src/ripple/rpc/impl/Handler.cpp index 2ba1c95c8..c4360b7a5 100644 --- a/src/ripple/rpc/impl/Handler.cpp +++ b/src/ripple/rpc/impl/Handler.cpp @@ -191,7 +191,8 @@ public: } Handler const* - getHandler(unsigned version, bool betaEnabled, std::string name) const + getHandler(unsigned version, bool betaEnabled, std::string const& name) + const { if (version < RPC::apiMinimumSupportedVersion || version > (betaEnabled ? RPC::apiBetaVersion diff --git a/src/test/basics/PerfLog_test.cpp b/src/test/basics/PerfLog_test.cpp index db0cf8851..0f9005d62 100644 --- a/src/test/basics/PerfLog_test.cpp +++ b/src/test/basics/PerfLog_test.cpp @@ -52,7 +52,7 @@ class PerfLog_test : public beast::unit_test::suite beast::Journal j_; bool stopSignaled{false}; - Fixture(beast::Journal const& j) : j_(j) + explicit Fixture(beast::Journal j) : j_(j) { } diff --git a/src/test/nodestore/DatabaseShard_test.cpp b/src/test/nodestore/DatabaseShard_test.cpp index d2ae7c7cf..e6f1e3e15 100644 --- a/src/test/nodestore/DatabaseShard_test.cpp +++ b/src/test/nodestore/DatabaseShard_test.cpp @@ -1193,7 +1193,7 @@ class DatabaseShard_test : public TestBase for (std::uint32_t i = 1; i <= 2; ++i) waitShard(*db, i); - auto const finalShards{std::move(db->getShardInfo()->finalized())}; + auto const finalShards{db->getShardInfo()->finalized()}; for (std::uint32_t shardIndex : {1, 2}) BEAST_EXPECT(boost::icl::contains(finalShards, shardIndex)); } @@ -1209,7 +1209,7 @@ class DatabaseShard_test : public TestBase for (std::uint32_t i = 1; i <= 2; ++i) waitShard(*db, i); - auto const finalShards{std::move(db->getShardInfo()->finalized())}; + auto const finalShards{db->getShardInfo()->finalized()}; for (std::uint32_t shardIndex : {1, 2}) BEAST_EXPECT(boost::icl::contains(finalShards, shardIndex)); @@ -1358,7 +1358,7 @@ class DatabaseShard_test : public TestBase for (std::uint32_t i = 1; i <= shardCount; ++i) waitShard(*db, i); - auto const final{std::move(db->getShardInfo()->finalized())}; + auto const final{db->getShardInfo()->finalized()}; for (std::uint32_t shardIndex : {1, 2, 3, 4}) BEAST_EXPECT(boost::icl::contains(final, shardIndex)); @@ -1417,7 +1417,7 @@ class DatabaseShard_test : public TestBase for (std::uint32_t i = 1; i <= shardCount; ++i) waitShard(*db, i); - auto const finalShards{std::move(db->getShardInfo()->finalized())}; + auto const finalShards{db->getShardInfo()->finalized()}; for (std::uint32_t shardIndex : {1, 2, 3, 4}) BEAST_EXPECT(boost::icl::contains(finalShards, shardIndex));