Skip to content

Commit

Permalink
sentry: treat clang-tidy warnings as errors
Browse files Browse the repository at this point in the history
  • Loading branch information
yperbasis committed Jan 3, 2024
1 parent fc6f7b9 commit 7653ef0
Show file tree
Hide file tree
Showing 9 changed files with 30 additions and 21 deletions.
4 changes: 4 additions & 0 deletions silkworm/sentry/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ find_package(Microsoft.GSL REQUIRED)
find_package(OpenSSL REQUIRED)
find_package(Snappy REQUIRED)

if(SILKWORM_CLANG_TIDY)
set(CMAKE_CXX_CLANG_TIDY "${CLANG_TIDY};-warnings-as-errors=*")
endif()

# sentry common
add_subdirectory(common)

Expand Down
14 changes: 8 additions & 6 deletions silkworm/sentry/discovery/discovery.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class DiscoveryImpl {
concurrency::ExecutorPool& executor_pool,
std::vector<EnodeUrl> peer_urls,
bool with_dynamic_discovery,
const std::filesystem::path& data_dir_path,
std::filesystem::path data_dir_path,
uint64_t network_id,
std::function<EccKeyPair()> node_key,
std::function<EnodeUrl()> node_url,
Expand Down Expand Up @@ -77,21 +77,21 @@ DiscoveryImpl::DiscoveryImpl(
concurrency::ExecutorPool& executor_pool,
std::vector<EnodeUrl> peer_urls,
bool with_dynamic_discovery,
const std::filesystem::path& data_dir_path,
std::filesystem::path data_dir_path,
uint64_t network_id,
std::function<EccKeyPair()> node_key,
std::function<EccKeyPair()> node_key, // NOLINT(performance-unnecessary-value-param)
std::function<EnodeUrl()> node_url,
std::function<enr::EnrRecord()> node_record,
std::vector<EnodeUrl> bootnodes,
uint16_t disc_v4_port)
: peer_urls_(std::move(peer_urls)),
with_dynamic_discovery_(with_dynamic_discovery),
data_dir_path_(data_dir_path),
data_dir_path_(std::move(data_dir_path)),
node_id_([node_key] { return node_key().public_key(); }),
network_id_(network_id),
node_db_(executor_pool.any_executor()),
bootnodes_(std::move(bootnodes)),
disc_v4_discovery_(executor_pool.any_executor(), disc_v4_port, node_key, node_url, node_record, node_db_.interface()) {
disc_v4_discovery_(executor_pool.any_executor(), disc_v4_port, node_key, std::move(node_url), std::move(node_record), node_db_.interface()) {
}

Task<void> DiscoveryImpl::run() {
Expand Down Expand Up @@ -139,8 +139,10 @@ Task<std::vector<Discovery::PeerCandidate>> DiscoveryImpl::request_peer_candidat
using namespace std::chrono_literals;

std::vector<node_db::NodeId> exclude_ids;
for (auto& url : exclude_urls)
exclude_ids.reserve(exclude_urls.size());
for (auto& url : exclude_urls) {
exclude_ids.push_back(url.public_key());
}

auto now = std::chrono::system_clock::now();
node_db::NodeDb::FindPeerCandidatesQuery query{
Expand Down
6 changes: 3 additions & 3 deletions silkworm/sentry/grpc/interfaces/peer_info.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@
namespace silkworm::sentry::grpc::interfaces {

api::PeerInfo peer_info_from_proto_peer_info(const types::PeerInfo& info);
types::PeerInfo proto_peer_info_from_peer_info(const api::PeerInfo& info);
types::PeerInfo proto_peer_info_from_peer_info(const api::PeerInfo&);

api::PeerInfos peer_infos_from_proto_peers_reply(const ::sentry::PeersReply& reply);
::sentry::PeersReply proto_peers_reply_from_peer_infos(const api::PeerInfos& infos);
::sentry::PeersReply proto_peers_reply_from_peer_infos(const api::PeerInfos&);

std::optional<api::PeerInfo> peer_info_opt_from_proto_peer_reply(const ::sentry::PeerByIdReply& reply);
::sentry::PeerByIdReply proto_peer_reply_from_peer_info_opt(const std::optional<api::PeerInfo>& info_opt);
::sentry::PeerByIdReply proto_peer_reply_from_peer_info_opt(const std::optional<api::PeerInfo>&);

} // namespace silkworm::sentry::grpc::interfaces
2 changes: 1 addition & 1 deletion silkworm/sentry/message_sender.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ Task<void> MessageSender::run(PeerManager& peer_manager) {

api::router::SendMessageCall::PeerKeys sent_peer_keys;

auto sender = [&message = call.message(), &sent_peer_keys, peer_filter = call.peer_filter()](std::shared_ptr<rlpx::Peer> peer) {
auto sender = [&message = call.message(), &sent_peer_keys, peer_filter = call.peer_filter()](const std::shared_ptr<rlpx::Peer>& peer) {
auto key_opt = peer->peer_public_key();
if (key_opt && (!peer_filter.peer_public_key || (key_opt.value() == peer_filter.peer_public_key.value()))) {
sent_peer_keys.push_back(key_opt.value());
Expand Down
4 changes: 2 additions & 2 deletions silkworm/sentry/multi_sentry_client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,13 @@ class MultiSentryClientImpl : public api::Service {
std::function<Task<void>(std::shared_ptr<api::Service>)> callback) {
using namespace concurrency::awaitable_wait_for_one;

std::function<Task<void>(size_t)> call_factory = [&clients, &callback](size_t index) -> Task<void> {
auto call_factory = [&clients, &callback](size_t index) -> Task<void> {
auto client = clients[index];
auto service = co_await client->service();
co_await callback(service);
};

auto group_task = concurrency::generate_parallel_group_task(clients.size(), std::move(call_factory));
auto group_task = concurrency::generate_parallel_group_task(clients.size(), call_factory);

try {
co_await (std::move(group_task) || concurrency::timeout(timeout));
Expand Down
6 changes: 3 additions & 3 deletions silkworm/sentry/peer_manager_api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ Task<void> PeerManagerApi::handle_peers_calls() {
auto call = co_await peers_calls_channel_.receive();

api::PeerInfos peers;
co_await peer_manager_.enumerate_peers([&peers](std::shared_ptr<rlpx::Peer> peer) {
co_await peer_manager_.enumerate_peers([&peers](const std::shared_ptr<rlpx::Peer>& peer) {
auto info_opt = make_peer_info(*peer);
if (info_opt) {
peers.push_back(info_opt.value());
Expand All @@ -109,7 +109,7 @@ Task<void> PeerManagerApi::handle_peer_calls() {
auto peer_public_key_opt = call.peer_public_key;

std::optional<api::PeerInfo> info_opt;
co_await peer_manager_.enumerate_peers([&info_opt, &peer_public_key_opt](std::shared_ptr<rlpx::Peer> peer) {
co_await peer_manager_.enumerate_peers([&info_opt, &peer_public_key_opt](const std::shared_ptr<rlpx::Peer>& peer) {
auto key_opt = peer->peer_public_key();
if (key_opt && peer_public_key_opt && (key_opt.value() == peer_public_key_opt.value())) {
info_opt = make_peer_info(*peer);
Expand All @@ -125,7 +125,7 @@ Task<void> PeerManagerApi::handle_peer_penalize_calls() {
while (true) {
auto peer_public_key_opt = co_await peer_penalize_calls_channel_.receive();

co_await peer_manager_.enumerate_peers([&peer_public_key_opt](std::shared_ptr<rlpx::Peer> peer) {
co_await peer_manager_.enumerate_peers([&peer_public_key_opt](const std::shared_ptr<rlpx::Peer>& peer) {
auto key_opt = peer->peer_public_key();
if (key_opt && peer_public_key_opt && (key_opt.value() == peer_public_key_opt.value())) {
peer->disconnect(rlpx::DisconnectReason::DisconnectRequested);
Expand Down
9 changes: 6 additions & 3 deletions silkworm/sentry/rlpx/auth/handshake.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@

#include <silkworm/infra/concurrency/task.hpp>

#include <absl/strings/str_cat.h>

#include <silkworm/sentry/common/ecc_key_pair.hpp>
#include <silkworm/sentry/common/ecc_public_key.hpp>
#include <silkworm/sentry/common/socket_stream.hpp>
Expand Down Expand Up @@ -71,9 +73,10 @@ class Handshake {
class CapabilityMismatchError : public std::runtime_error {
public:
CapabilityMismatchError(
std::string required_capability_desc,
std::string peer_capabilities_desc)
: std::runtime_error("rlpx::auth::Handshake: no matching required capability " + required_capability_desc + " in " + peer_capabilities_desc) {}
const std::string& required_capability_desc,
const std::string& peer_capabilities_desc)
: std::runtime_error(absl::StrCat("rlpx::auth::Handshake: no matching required capability ",
required_capability_desc, " in ", peer_capabilities_desc)) {}
};

private:
Expand Down
4 changes: 2 additions & 2 deletions silkworm/sentry/rlpx/peer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,8 @@ Task<void> Peer::drop(const std::shared_ptr<Peer>& peer, DisconnectReason reason
return concurrency::co_spawn_sw(peer->strand_, Peer::drop_in_strand(peer, reason), use_awaitable);
}

Task<void> Peer::drop_in_strand(std::shared_ptr<Peer> self, DisconnectReason reason) {
co_await self->drop(reason);
Task<void> Peer::drop_in_strand(std::shared_ptr<Peer> peer, DisconnectReason reason) {
co_await peer->drop(reason);
}

Task<void> Peer::drop(DisconnectReason reason) {
Expand Down
2 changes: 1 addition & 1 deletion silkworm/sentry/session_sentry_client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ class SessionSentryClientImpl : public api::SentryClient {
SessionSentryClient::SessionSentryClient(
std::shared_ptr<api::SentryClient> sentry_client,
StatusDataProvider status_data_provider)
: p_impl_(std::make_unique<SessionSentryClientImpl>(sentry_client, status_data_provider)) {
: p_impl_(std::make_unique<SessionSentryClientImpl>(std::move(sentry_client), std::move(status_data_provider))) {
}

SessionSentryClient::~SessionSentryClient() {
Expand Down

0 comments on commit 7653ef0

Please sign in to comment.