refactor: Enable clang-tidy bugprone-unused-return-value
What changed, and why it matters
This is a code cleanup change that turns on a static-analysis check (clang-tidy 'bugprone-unused-return-value') and makes small, behavior-preserving edits so the codebase passes the new check. It does not fix a security bug or change how the software behaves at runtime.
No security action needed; treat as normal refactoring/static-analysis hygiene.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit enables the clang-tidy check bugprone-unused-return-value and configures it to flag unused returns for std::error_code, std::error_condition, std::errc, std::expected, util::Result, and util::Expected. To satisfy the new lint, it adds [[maybe_unused]] auto _{…} wrappers around intentionally discarded return values in benchmarks, tests, and http server code. It also refactors an evhttp_make_request call to release the unique_ptr inside the argument list rather than on the next line, which is equivalent in effect. No functional or security-relevant behavior is changed.
Changed components
src/.clang-tidysrc/bench/coin_selection.cppsrc/bitcoin-cli.cppsrc/httpserver.cppsrc/ipc/test/ipc_test.cppsrc/wallet/test/fuzz/spend.cppInspect captured patch +10 / −6
diff --git a/src/.clang-tidy b/src/.clang-tidy
index 544bd5ac..da53a588 100644
--- a/src/.clang-tidy
+++ b/src/.clang-tidy
@@ -7,6 +7,7 @@ bugprone-string-constructor,
bugprone-use-after-move,
bugprone-lambda-function-name,
bugprone-unhandled-self-assignment,
+bugprone-unused-return-value,
misc-unused-using-decls,
misc-no-recursion,
modernize-deprecated-headers,
@@ -36,3 +37,5 @@ CheckOptions:
value: false
- key: bugprone-unhandled-self-assignment.WarnOnlyIfThisHasSuspiciousField
value: false
+ - key: bugprone-unused-return-value.CheckedReturnTypes
+ value: '^::std::error_code$;^::std::error_condition$;^::std::errc$;^::std::expected$;^::util::Result$;^::util::Expected$'
diff --git a/src/bench/coin_selection.cpp b/src/bench/coin_selection.cpp
index dfefa1b5..2150d800 100644
--- a/src/bench/coin_selection.cpp
+++ b/src/bench/coin_selection.cpp
@@ -130,7 +130,7 @@ static void BnBExhaustion(benchmark::Bench& bench)
bench.run([&] {
// Benchmark
CAmount target = make_hard_case(17, utxo_pool);
- SelectCoinsBnB(utxo_pool, target, 0, MAX_STANDARD_TX_WEIGHT); // Should exhaust
+ [[maybe_unused]] auto _{SelectCoinsBnB(utxo_pool, target, /*cost_of_change=*/0, MAX_STANDARD_TX_WEIGHT)}; // Should exhaust
// Cleanup
utxo_pool.clear();
diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp
index 279aa89e..d6fcaa84 100644
--- a/src/bitcoin-cli.cpp
+++ b/src/bitcoin-cli.cpp
@@ -899,8 +899,7 @@ static UniValue CallRPC(BaseRequestHandler* rh, const std::string& strMethod, co
throw CConnectionFailed("uri-encode failed");
}
}
- int r = evhttp_make_request(evcon.get(), req.get(), EVHTTP_REQ_POST, endpoint.c_str());
- req.release(); // ownership moved to evcon in above call
+ int r = evhttp_make_request(evcon.get(), req.release(), EVHTTP_REQ_POST, endpoint.c_str());
if (r != 0) {
throw CConnectionFailed("send http request failed");
}
diff --git a/src/httpserver.cpp b/src/httpserver.cpp
index 5644ddbc..abfcb455 100644
--- a/src/httpserver.cpp
+++ b/src/httpserver.cpp
@@ -330,7 +330,7 @@ static void http_request_cb(struct evhttp_request* req, void* arg)
std::unique_ptr<HTTPWorkItem> item(new HTTPWorkItem(std::move(hreq), path, i->handler));
assert(g_work_queue);
if (g_work_queue->Enqueue(item.get())) {
- item.release(); /* if true, queue took ownership */
+ [[maybe_unused]] auto _{item.release()}; /* if true, queue took ownership */
} else {
LogWarning("Request rejected because http work queue depth exceeded, it can be increased with the -rpcworkqueue= setting");
item->req->WriteReply(HTTP_SERVICE_UNAVAILABLE, "Work queue depth exceeded");
diff --git a/src/ipc/test/ipc_test.cpp b/src/ipc/test/ipc_test.cpp
index a350d60b..506facde 100644
--- a/src/ipc/test/ipc_test.cpp
+++ b/src/ipc/test/ipc_test.cpp
@@ -63,7 +63,9 @@ void IpcPipeTest()
auto foo_client = std::make_unique<mp::ProxyClient<gen::FooInterface>>(
connection_client->m_rpc_system->bootstrap(mp::ServerVatId().vat_id).castAs<gen::FooInterface>(),
connection_client.get(), /* destroy_connection= */ true);
- connection_client.release();
+ {
+ [[maybe_unused]] auto _{connection_client.release()};
+ }
foo_promise.set_value(std::move(foo_client));
auto connection_server = std::make_unique<mp::Connection>(loop, kj::mv(pipe.ends[1]), [&](mp::Connection& connection) {
diff --git a/src/wallet/test/fuzz/spend.cpp b/src/wallet/test/fuzz/spend.cpp
index 552364a6..99bc5345 100644
--- a/src/wallet/test/fuzz/spend.cpp
+++ b/src/wallet/test/fuzz/spend.cpp
@@ -98,7 +98,7 @@ FUZZ_TARGET(wallet_create_transaction, .init = initialize_setup)
std::optional<unsigned int> change_pos;
if (fuzzed_data_provider.ConsumeBool()) change_pos = fuzzed_data_provider.ConsumeIntegral<unsigned int>();
- (void)CreateTransaction(*fuzzed_wallet.wallet, recipients, change_pos, coin_control);
+ [[maybe_unused]] auto _{CreateTransaction(*fuzzed_wallet.wallet, recipients, change_pos, coin_control)};
}
} // namespace
} // namespace wallet
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.