From afc660a1b5764536cf40350714ee5849ac60a067 Mon Sep 17 00:00:00 2001 From: Alex Kremer Date: Mon, 2 Mar 2026 17:08:56 +0000 Subject: [PATCH] refactor: Fix clang-tidy `bugprone-empty-catch` check (#6419) This change fixes or suppresses instances detected by the `bugprone-empty-catch` clang-tidy check. --- .clang-tidy | 2 +- cspell.config.yaml | 2 ++ src/libxrpl/beast/insight/StatsDCollector.cpp | 2 +- src/libxrpl/nodestore/backend/NuDBFactory.cpp | 2 +- src/libxrpl/protocol/STAmount.cpp | 8 ++++---- src/libxrpl/protocol/STTx.cpp | 4 ++-- src/libxrpl/tx/transactors/XChainBridge.cpp | 2 +- src/test/app/Manifest_test.cpp | 4 ++-- src/test/core/SociDB_test.cpp | 4 ++-- src/test/jtx/impl/Env.cpp | 8 ++++---- src/test/jtx/impl/Oracle.cpp | 2 +- src/test/jtx/impl/WSClient.cpp | 1 + src/tests/libxrpl/basics/scope.cpp | 12 ++++++------ src/xrpld/app/ledger/detail/InboundLedgers.cpp | 2 +- src/xrpld/app/ledger/detail/LedgerMaster.cpp | 2 +- src/xrpld/app/ledger/detail/SkipListAcquire.cpp | 2 +- src/xrpld/app/main/GRPCServer.cpp | 2 +- src/xrpld/app/misc/detail/ValidatorSite.cpp | 4 ++-- src/xrpld/overlay/detail/ConnectAttempt.cpp | 2 +- src/xrpld/rpc/detail/RPCCall.cpp | 2 +- 20 files changed, 36 insertions(+), 33 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 5f4187b008..5971b5dd14 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -10,6 +10,7 @@ Checks: "-*, bugprone-copy-constructor-init, bugprone-dangling-handle, bugprone-dynamic-static-initializers, + bugprone-empty-catch, bugprone-fold-init-type, bugprone-forward-declaration-namespace, bugprone-inaccurate-erase, @@ -83,7 +84,6 @@ Checks: "-*, # --- # checks that have some issues that need to be resolved: # -# bugprone-empty-catch, # bugprone-crtp-constructor-accessibility, # bugprone-inc-dec-in-conditions, # bugprone-reserved-identifier, diff --git a/cspell.config.yaml b/cspell.config.yaml index e2b20ac098..98b6be81e7 100644 --- a/cspell.config.yaml +++ b/cspell.config.yaml @@ -176,6 +176,8 @@ words: - nixfmt - nixos - nixpkgs + - NOLINT + - NOLINTNEXTLINE - nonxrp - noripple - nudb diff --git a/src/libxrpl/beast/insight/StatsDCollector.cpp b/src/libxrpl/beast/insight/StatsDCollector.cpp index 8462a00b3d..143bc51bd8 100644 --- a/src/libxrpl/beast/insight/StatsDCollector.cpp +++ b/src/libxrpl/beast/insight/StatsDCollector.cpp @@ -249,7 +249,7 @@ public: { m_timer.cancel(); } - catch (boost::system::system_error const&) + catch (boost::system::system_error const&) // NOLINT(bugprone-empty-catch) { // ignored } diff --git a/src/libxrpl/nodestore/backend/NuDBFactory.cpp b/src/libxrpl/nodestore/backend/NuDBFactory.cpp index 4d7e7be668..c79938bcf8 100644 --- a/src/libxrpl/nodestore/backend/NuDBFactory.cpp +++ b/src/libxrpl/nodestore/backend/NuDBFactory.cpp @@ -83,7 +83,7 @@ public: // close can throw and we don't want the destructor to throw. close(); } - catch (nudb::system_error const&) + catch (nudb::system_error const&) // NOLINT(bugprone-empty-catch) { // Don't allow exceptions to propagate out of destructors. // close() has already logged the error. diff --git a/src/libxrpl/protocol/STAmount.cpp b/src/libxrpl/protocol/STAmount.cpp index 650cc4369d..9503da57a2 100644 --- a/src/libxrpl/protocol/STAmount.cpp +++ b/src/libxrpl/protocol/STAmount.cpp @@ -443,6 +443,7 @@ getRate(STAmount const& offerOut, STAmount const& offerIn) { if (offerOut == beast::zero) return 0; + try { STAmount r = divide(offerIn, offerOut, noIssue()); @@ -454,12 +455,11 @@ getRate(STAmount const& offerOut, STAmount const& offerIn) std::uint64_t ret = r.exponent() + 100; return (ret << (64 - 8)) | r.mantissa(); } - catch (std::exception const&) + catch (...) { + // overflow -- very bad offer + return 0; } - - // overflow -- very bad offer - return 0; } /** diff --git a/src/libxrpl/protocol/STTx.cpp b/src/libxrpl/protocol/STTx.cpp index 0c5e299702..098ca1a400 100644 --- a/src/libxrpl/protocol/STTx.cpp +++ b/src/libxrpl/protocol/STTx.cpp @@ -246,10 +246,10 @@ STTx::checkSign(Rules const& rules, STObject const& sigObject) const return signingPubKey.empty() ? checkMultiSign(rules, sigObject) : checkSingleSign(sigObject); } - catch (std::exception const&) + catch (...) { + return Unexpected("Internal signature check failure."); } - return Unexpected("Internal signature check failure."); } Expected diff --git a/src/libxrpl/tx/transactors/XChainBridge.cpp b/src/libxrpl/tx/transactors/XChainBridge.cpp index 30fc9f59e1..64daa6d1ee 100644 --- a/src/libxrpl/tx/transactors/XChainBridge.cpp +++ b/src/libxrpl/tx/transactors/XChainBridge.cpp @@ -1126,8 +1126,8 @@ toClaim(STTx const& tx) } catch (...) { + return std::nullopt; } - return std::nullopt; } template diff --git a/src/test/app/Manifest_test.cpp b/src/test/app/Manifest_test.cpp index a790584ac2..294d5210d9 100644 --- a/src/test/app/Manifest_test.cpp +++ b/src/test/app/Manifest_test.cpp @@ -71,7 +71,7 @@ public: { setupDatabaseDir(getDatabasePath()); } - catch (std::exception const&) + catch (std::exception const&) // NOLINT(bugprone-empty-catch) { } } @@ -81,7 +81,7 @@ public: { cleanupDatabaseDir(getDatabasePath()); } - catch (std::exception const&) + catch (std::exception const&) // NOLINT(bugprone-empty-catch) { } } diff --git a/src/test/core/SociDB_test.cpp b/src/test/core/SociDB_test.cpp index a06193ae86..66b368176d 100644 --- a/src/test/core/SociDB_test.cpp +++ b/src/test/core/SociDB_test.cpp @@ -58,7 +58,7 @@ public: { setupDatabaseDir(getDatabasePath()); } - catch (std::exception const&) + catch (std::exception const&) // NOLINT(bugprone-empty-catch) { } } @@ -68,7 +68,7 @@ public: { cleanupDatabaseDir(getDatabasePath()); } - catch (std::exception const&) + catch (std::exception const&) // NOLINT(bugprone-empty-catch) { } } diff --git a/src/test/jtx/impl/Env.cpp b/src/test/jtx/impl/Env.cpp index df86aaa2e4..4dfd2f2b38 100644 --- a/src/test/jtx/impl/Env.cpp +++ b/src/test/jtx/impl/Env.cpp @@ -587,10 +587,10 @@ Env::st(JTx const& jt) { return sterilize(STTx{std::move(*obj)}); } - catch (std::exception const&) + catch (...) { + return nullptr; } - return nullptr; } std::shared_ptr @@ -613,10 +613,10 @@ Env::ust(JTx const& jt) { return std::make_shared(std::move(*obj)); } - catch (std::exception const&) + catch (...) { + return nullptr; } - return nullptr; } Json::Value diff --git a/src/test/jtx/impl/Oracle.cpp b/src/test/jtx/impl/Oracle.cpp index c9d8c0ce27..302880c972 100644 --- a/src/test/jtx/impl/Oracle.cpp +++ b/src/test/jtx/impl/Oracle.cpp @@ -339,8 +339,8 @@ validDocumentID(AnyValue const& v) } catch (...) { + return false; } - return false; } } // namespace oracle diff --git a/src/test/jtx/impl/WSClient.cpp b/src/test/jtx/impl/WSClient.cpp index 2b92eb5ec3..84424be222 100644 --- a/src/test/jtx/impl/WSClient.cpp +++ b/src/test/jtx/impl/WSClient.cpp @@ -107,6 +107,7 @@ class WSClientImpl : public WSClient { stream_.cancel(); } + // NOLINTNEXTLINE(bugprone-empty-catch) catch (boost::system::system_error const&) { // ignored diff --git a/src/tests/libxrpl/basics/scope.cpp b/src/tests/libxrpl/basics/scope.cpp index 309a41ec04..8efa4a84b1 100644 --- a/src/tests/libxrpl/basics/scope.cpp +++ b/src/tests/libxrpl/basics/scope.cpp @@ -35,7 +35,7 @@ TEST(scope, scope_exit) scope_exit x{[&i]() { i = 5; }}; throw 1; } - catch (...) + catch (...) // NOLINT(bugprone-empty-catch) { } } @@ -47,7 +47,7 @@ TEST(scope, scope_exit) x.release(); throw 1; } - catch (...) + catch (...) // NOLINT(bugprone-empty-catch) { } } @@ -85,7 +85,7 @@ TEST(scope, scope_fail) scope_fail x{[&i]() { i = 5; }}; throw 1; } - catch (...) + catch (...) // NOLINT(bugprone-empty-catch) { } } @@ -97,7 +97,7 @@ TEST(scope, scope_fail) x.release(); throw 1; } - catch (...) + catch (...) // NOLINT(bugprone-empty-catch) { } } @@ -135,7 +135,7 @@ TEST(scope, scope_success) scope_success x{[&i]() { i = 5; }}; throw 1; } - catch (...) + catch (...) // NOLINT(bugprone-empty-catch) { } } @@ -147,7 +147,7 @@ TEST(scope, scope_success) x.release(); throw 1; } - catch (...) + catch (...) // NOLINT(bugprone-empty-catch) { } } diff --git a/src/xrpld/app/ledger/detail/InboundLedgers.cpp b/src/xrpld/app/ledger/detail/InboundLedgers.cpp index a8ae530bde..e17437d64f 100644 --- a/src/xrpld/app/ledger/detail/InboundLedgers.cpp +++ b/src/xrpld/app/ledger/detail/InboundLedgers.cpp @@ -241,7 +241,7 @@ public: newNode->getHash().as_uint256(), std::make_shared(s.begin(), s.end())); } } - catch (std::exception const&) + catch (std::exception const&) // NOLINT(bugprone-empty-catch) { } } diff --git a/src/xrpld/app/ledger/detail/LedgerMaster.cpp b/src/xrpld/app/ledger/detail/LedgerMaster.cpp index 8072b619e1..64bdf04df1 100644 --- a/src/xrpld/app/ledger/detail/LedgerMaster.cpp +++ b/src/xrpld/app/ledger/detail/LedgerMaster.cpp @@ -1637,7 +1637,7 @@ LedgerMaster::getLedgerBySeq(std::uint32_t index) if (hash) return mLedgerHistory.getLedgerByHash(*hash); } - catch (std::exception const&) + catch (std::exception const&) // NOLINT(bugprone-empty-catch) { // Missing nodes are already handled } diff --git a/src/xrpld/app/ledger/detail/SkipListAcquire.cpp b/src/xrpld/app/ledger/detail/SkipListAcquire.cpp index 0fb1239c49..2191ef965a 100644 --- a/src/xrpld/app/ledger/detail/SkipListAcquire.cpp +++ b/src/xrpld/app/ledger/detail/SkipListAcquire.cpp @@ -127,7 +127,7 @@ SkipListAcquire::processData( return; } } - catch (...) + catch (...) // NOLINT(bugprone-empty-catch) { } diff --git a/src/xrpld/app/main/GRPCServer.cpp b/src/xrpld/app/main/GRPCServer.cpp index ced252cb71..c6b5c91e14 100644 --- a/src/xrpld/app/main/GRPCServer.cpp +++ b/src/xrpld/app/main/GRPCServer.cpp @@ -29,7 +29,7 @@ getEndpoint(std::string const& peer) if (endpoint) return beast::IP::to_asio_endpoint(endpoint.value()); } - catch (std::exception const&) + catch (std::exception const&) // NOLINT(bugprone-empty-catch) { } return {}; diff --git a/src/xrpld/app/misc/detail/ValidatorSite.cpp b/src/xrpld/app/misc/detail/ValidatorSite.cpp index fb68bf5ef4..c4077a1b8b 100644 --- a/src/xrpld/app/misc/detail/ValidatorSite.cpp +++ b/src/xrpld/app/misc/detail/ValidatorSite.cpp @@ -177,7 +177,7 @@ ValidatorSite::stop() { timer_.cancel(); } - catch (boost::system::system_error const&) + catch (boost::system::system_error const&) // NOLINT(bugprone-empty-catch) { } stopping_ = false; @@ -222,7 +222,7 @@ ValidatorSite::makeRequest( { timer_.cancel_one(); } - catch (boost::system::system_error const&) + catch (boost::system::system_error const&) // NOLINT(bugprone-empty-catch) { } }; diff --git a/src/xrpld/overlay/detail/ConnectAttempt.cpp b/src/xrpld/overlay/detail/ConnectAttempt.cpp index c9361a2a5d..ac0743e936 100644 --- a/src/xrpld/overlay/detail/ConnectAttempt.cpp +++ b/src/xrpld/overlay/detail/ConnectAttempt.cpp @@ -252,7 +252,7 @@ ConnectAttempt::cancelTimer() timer_.cancel(); stepTimer_.cancel(); } - catch (boost::system::system_error const&) + catch (boost::system::system_error const&) // NOLINT(bugprone-empty-catch) { // ignored } diff --git a/src/xrpld/rpc/detail/RPCCall.cpp b/src/xrpld/rpc/detail/RPCCall.cpp index 7b65daa839..134cbb34f8 100644 --- a/src/xrpld/rpc/detail/RPCCall.cpp +++ b/src/xrpld/rpc/detail/RPCCall.cpp @@ -1479,7 +1479,7 @@ rpcClient( setup = setup_ServerHandler( config, beast::logstream{logs.journal("HTTPClient").warn()}); } - catch (std::exception const&) + catch (std::exception const&) // NOLINT(bugprone-empty-catch) { // ignore any exceptions, so the command // line client works without a config file