From 82484e26f57719199f67870dc21f10dcf82adb14 Mon Sep 17 00:00:00 2001 From: Mike Ellery Date: Thu, 3 Oct 2019 10:17:15 -0700 Subject: [PATCH] Add option to enable -Wextra for gcc/clang. --- .travis.yml | 2 +- Builds/CMake/RippledCompiler.cmake | 3 +++ Builds/CMake/RippledSettings.cmake | 2 ++ src/ripple/app/ledger/Ledger.cpp | 4 +--- src/ripple/app/main/Application.cpp | 2 +- src/ripple/app/misc/impl/Manifest.cpp | 2 +- src/ripple/app/misc/impl/Transaction.cpp | 3 ++- src/ripple/beast/rfc2616.h | 4 ++++ src/ripple/overlay/PeerReservationTable.h | 2 +- src/ripple/protocol/impl/STParsedJSON.cpp | 2 +- src/ripple/rpc/Context.h | 6 +++--- src/ripple/rpc/impl/TransactionSign.cpp | 8 ++++---- src/ripple/shamap/impl/SHAMapTreeNode.cpp | 2 +- src/test/app/LedgerLoad_test.cpp | 6 +++--- src/test/app/Path_test.cpp | 6 +++--- 15 files changed, 31 insertions(+), 23 deletions(-) diff --git a/.travis.yml b/.travis.yml index bd4dfe7363..7ac33241a9 100644 --- a/.travis.yml +++ b/.travis.yml @@ -7,7 +7,7 @@ services: env: global: - DOCKER_IMAGE="mellery451/rippled-ci-builder:2019-08-26" - - CMAKE_EXTRA_ARGS="-Dwerr=ON" + - CMAKE_EXTRA_ARGS="-Dwerr=ON -Dwextra=ON" - NINJA_BUILD=true # change this if we get more VM capacity - MAX_TIME_MIN=80 diff --git a/Builds/CMake/RippledCompiler.cmake b/Builds/CMake/RippledCompiler.cmake index 995b519d7c..a289c97226 100644 --- a/Builds/CMake/RippledCompiler.cmake +++ b/Builds/CMake/RippledCompiler.cmake @@ -87,6 +87,9 @@ if (MSVC) else () # HACK : because these need to come first, before any warning demotion string (APPEND CMAKE_CXX_FLAGS " -Wall -Wdeprecated") + if (wextra) + string (APPEND CMAKE_CXX_FLAGS " -Wextra -Wno-unused-parameter") + endif () # not MSVC target_compile_options (common INTERFACE diff --git a/Builds/CMake/RippledSettings.cmake b/Builds/CMake/RippledSettings.cmake index fb89738107..edfdab9cf8 100644 --- a/Builds/CMake/RippledSettings.cmake +++ b/Builds/CMake/RippledSettings.cmake @@ -16,9 +16,11 @@ if (is_gcc OR is_clang) "Include only src/ripple files when generating coverage report. \ Set to OFF to include all sources in coverage report." ON) + option (wextra "compile with extra gcc/clang warnings enabled" ON) else () set (profile OFF CACHE BOOL "gcc/clang only" FORCE) set (coverage OFF CACHE BOOL "gcc/clang only" FORCE) + set (wextra OFF CACHE BOOL "gcc/clang only" FORCE) endif () if (is_linux) option (BUILD_SHARED_LIBS "build shared ripple libraries" OFF) diff --git a/src/ripple/app/ledger/Ledger.cpp b/src/ripple/app/ledger/Ledger.cpp index a1cff92b5a..6ac6eef584 100644 --- a/src/ripple/app/ledger/Ledger.cpp +++ b/src/ripple/app/ledger/Ledger.cpp @@ -433,9 +433,7 @@ Ledger::read (Keylet const& k) const item->size()}, item->key()); if (! k.check(*sle)) return nullptr; - // need move otherwise makes a copy - // because return type is different - return std::move(sle); + return sle; } //------------------------------------------------------------------------------ diff --git a/src/ripple/app/main/Application.cpp b/src/ripple/app/main/Application.cpp index 8b22320582..59427000de 100644 --- a/src/ripple/app/main/Application.cpp +++ b/src/ripple/app/main/Application.cpp @@ -1567,7 +1567,7 @@ bool ApplicationImp::setup() Resource::Charge loadType = Resource::feeReferenceRPC; Resource::Consumer c; RPC::Context context { journal ("RPCHandler"), jvCommand, *this, - loadType, getOPs (), getLedgerMaster(), c, Role::ADMIN }; + loadType, getOPs (), getLedgerMaster (), c, Role::ADMIN}; Json::Value jvResult; RPC::doCommand (context, jvResult); diff --git a/src/ripple/app/misc/impl/Manifest.cpp b/src/ripple/app/misc/impl/Manifest.cpp index c64571dfad..123254d2bf 100644 --- a/src/ripple/app/misc/impl/Manifest.cpp +++ b/src/ripple/app/misc/impl/Manifest.cpp @@ -146,7 +146,7 @@ boost::optional deserializeManifest(Slice s) m.signingKey = PublicKey(makeSlice(spk)); } - return std::move(m); + return m; } catch (std::exception const&) { diff --git a/src/ripple/app/misc/impl/Transaction.cpp b/src/ripple/app/misc/impl/Transaction.cpp index 1f84e7c494..a31b3c04fc 100644 --- a/src/ripple/app/misc/impl/Transaction.cpp +++ b/src/ripple/app/misc/impl/Transaction.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -63,7 +64,7 @@ void Transaction::setStatus (TransStatus ts, std::uint32_t lseq) TransStatus Transaction::sqlTransactionStatus( boost::optional const& status) { - char const c = (status) ? (*status)[0] : txnSqlUnknown; + char const c = (status) ? (*status)[0] : safe_cast(txnSqlUnknown); switch (c) { diff --git a/src/ripple/beast/rfc2616.h b/src/ripple/beast/rfc2616.h index b7d9fb8747..4a970e1c33 100644 --- a/src/ripple/beast/rfc2616.h +++ b/src/ripple/beast/rfc2616.h @@ -112,7 +112,11 @@ inline bool is_char(char c) { +#ifdef __CHAR_UNSIGNED__ /* -funsigned-char */ return c >= 0 && c <= 127; +#else + return c >= 0; +#endif } template diff --git a/src/ripple/overlay/PeerReservationTable.h b/src/ripple/overlay/PeerReservationTable.h index 006ede698a..cb0819dcee 100644 --- a/src/ripple/overlay/PeerReservationTable.h +++ b/src/ripple/overlay/PeerReservationTable.h @@ -44,7 +44,7 @@ struct PeerReservation final { public: PublicKey nodeId; - std::string description; + std::string description {}; auto toJson() const -> Json::Value; diff --git a/src/ripple/protocol/impl/STParsedJSON.cpp b/src/ripple/protocol/impl/STParsedJSON.cpp index 8a13e7b596..c8795c43d4 100644 --- a/src/ripple/protocol/impl/STParsedJSON.cpp +++ b/src/ripple/protocol/impl/STParsedJSON.cpp @@ -819,7 +819,7 @@ static boost::optional parseObject ( // Some inner object types have templates. Attempt to apply that. data.applyTemplateFromSField (inName); // May throw - return std::move (data); + return data; } catch (STObject::FieldErr const&) { diff --git a/src/ripple/rpc/Context.h b/src/ripple/rpc/Context.h index 20a8a574e3..07e0da3f18 100644 --- a/src/ripple/rpc/Context.h +++ b/src/ripple/rpc/Context.h @@ -55,9 +55,9 @@ struct Context LedgerMaster& ledgerMaster; Resource::Consumer& consumer; Role role; - std::shared_ptr coro; - InfoSub::pointer infoSub; - Headers headers; + std::shared_ptr coro {}; + InfoSub::pointer infoSub {}; + Headers headers {}; }; } // RPC diff --git a/src/ripple/rpc/impl/TransactionSign.cpp b/src/ripple/rpc/impl/TransactionSign.cpp index e7edf9ef2e..266596a69d 100644 --- a/src/ripple/rpc/impl/TransactionSign.cpp +++ b/src/ripple/rpc/impl/TransactionSign.cpp @@ -353,7 +353,7 @@ transactionPreProcessImpl ( Json::Value jvResult; auto const [pk, sk] = keypairForSignature (params, jvResult); if (contains_error (jvResult)) - return std::move (jvResult); + return jvResult; bool const verify = !(params.isMember (jss::offline) && params[jss::offline].asBool()); @@ -402,7 +402,7 @@ transactionPreProcessImpl ( ledger); if (RPC::contains_error (err)) - return std::move (err); + return err; err = checkPayment ( params, @@ -414,7 +414,7 @@ transactionPreProcessImpl ( verify && signingArgs.editFields()); if (RPC::contains_error(err)) - return std::move (err); + return err; } if (signingArgs.editFields()) @@ -495,7 +495,7 @@ transactionPreProcessImpl ( err [jss::error] = parsed.error [jss::error]; err [jss::error_code] = parsed.error [jss::error_code]; err [jss::error_message] = parsed.error [jss::error_message]; - return std::move (err); + return err; } std::shared_ptr stpTrans; diff --git a/src/ripple/shamap/impl/SHAMapTreeNode.cpp b/src/ripple/shamap/impl/SHAMapTreeNode.cpp index efa832aee3..4c66871b40 100644 --- a/src/ripple/shamap/impl/SHAMapTreeNode.cpp +++ b/src/ripple/shamap/impl/SHAMapTreeNode.cpp @@ -46,7 +46,7 @@ SHAMapInnerNode::clone(std::uint32_t seq) const std::lock_guard lock(childLock); for (int i = 0; i < 16; ++i) p->mChildren[i] = mChildren[i]; - return std::move(p); + return p; } std::shared_ptr diff --git a/src/test/app/LedgerLoad_test.cpp b/src/test/app/LedgerLoad_test.cpp index 799da369f1..58cd39dc39 100644 --- a/src/test/app/LedgerLoad_test.cpp +++ b/src/test/app/LedgerLoad_test.cpp @@ -48,9 +48,9 @@ class LedgerLoad_test : public beast::unit_test::suite struct SetupData { std::string const dbPath; - std::string ledgerFile; - Json::Value ledger; - Json::Value hashes; + std::string ledgerFile {}; + Json::Value ledger {}; + Json::Value hashes {}; }; SetupData diff --git a/src/test/app/Path_test.cpp b/src/test/app/Path_test.cpp index 6965237fa9..1d38d54e2a 100644 --- a/src/test/app/Path_test.cpp +++ b/src/test/app/Path_test.cpp @@ -220,8 +220,8 @@ public: auto& app = env.app(); Resource::Charge loadType = Resource::feeReferenceRPC; Resource::Consumer c; - RPC::Context context {env.journal, {}, app, loadType, - app.getOPs(), app.getLedgerMaster(), c, Role::USER, {}}; + RPC::Context context { env.journal, {}, app, loadType, + app.getOPs(), app.getLedgerMaster(), c, Role::USER}; Json::Value params = Json::objectValue; params[jss::command] = "ripple_path_find"; @@ -320,7 +320,7 @@ public: Resource::Charge loadType = Resource::feeReferenceRPC; Resource::Consumer c; RPC::Context context {env.journal, {}, app, loadType, - app.getOPs(), app.getLedgerMaster(), c, Role::USER, {}}; + app.getOPs(), app.getLedgerMaster(), c, Role::USER}; Json::Value result; gate g; // Test RPC::Tuning::max_src_cur source currencies.