From 60ef9e072f011c48cdd95ec74d6af0375a5d1e74 Mon Sep 17 00:00:00 2001 From: yperbasis Date: Wed, 17 Jan 2024 10:55:54 +0100 Subject: [PATCH] Address concurrency-mt-unsafe warnings --- .clang-tidy | 3 +-- silkworm/infra/common/directories.cpp | 5 +++-- silkworm/infra/concurrency/parallel_group_test.cpp | 6 +++--- silkworm/node/bittorrent/client.cpp | 9 ++++++++- 4 files changed, 15 insertions(+), 8 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 69c5b09b41..eb53607172 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,5 +1,5 @@ FormatStyle: file -HeaderFilterRegex: 'silkworm/(api|core|infra|node|sentry|rpc|sync)/.+\.hpp$' +HeaderFilterRegex: 'silkworm/(capi|core|infra|node|rpc|sentry|sync|wasm)/.+\.hpp$' WarningsAsErrors: '*' Checks: > abseil-*, @@ -15,7 +15,6 @@ Checks: > -clang-analyzer-*, clang-diagnostic-*, concurrency-*, - -concurrency-mt-unsafe, cppcoreguidelines-*, -cppcoreguidelines-avoid-c-arrays, -cppcoreguidelines-avoid-const-or-ref-data-members, diff --git a/silkworm/infra/common/directories.cpp b/silkworm/infra/common/directories.cpp index 60a2e285f5..8c406d6237 100644 --- a/silkworm/infra/common/directories.cpp +++ b/silkworm/infra/common/directories.cpp @@ -138,7 +138,8 @@ DataDirectory DataDirectory::from_chaindata(const std::filesystem::path& chainda std::filesystem::path silkworm::DataDirectory::get_default_storage_path() { std::string base_dir_str{}; - const char* env{std::getenv("XDG_DATA_HOME")}; + // C++11 guarantees some thread safety for std::getenv + const char* env{std::getenv("XDG_DATA_HOME")}; // NOLINT(concurrency-mt-unsafe) if (env) { // Got storage path from docker base_dir_str.assign(env); @@ -148,7 +149,7 @@ std::filesystem::path silkworm::DataDirectory::get_default_storage_path() { #else std::string env_name{"HOME"}; #endif - env = std::getenv(env_name.c_str()); + env = std::getenv(env_name.c_str()); // NOLINT(concurrency-mt-unsafe) if (!env) { // We don't actually know where to store data // fallback to current directory diff --git a/silkworm/infra/concurrency/parallel_group_test.cpp b/silkworm/infra/concurrency/parallel_group_test.cpp index 5bc576b10c..9d858f1bb8 100644 --- a/silkworm/infra/concurrency/parallel_group_test.cpp +++ b/silkworm/infra/concurrency/parallel_group_test.cpp @@ -35,7 +35,7 @@ using namespace boost::asio::experimental; using namespace silkworm::concurrency; using namespace std::chrono_literals; -awaitable sleep(std::chrono::milliseconds duration) { +awaitable my_sleep(std::chrono::milliseconds duration) { auto executor = co_await this_coro::executor; steady_timer timer(executor); timer.expires_after(duration); @@ -47,7 +47,7 @@ awaitable noop() { } awaitable throw_op() { - co_await sleep(1ms); + co_await my_sleep(1ms); throw std::runtime_error("throw_op"); } @@ -67,7 +67,7 @@ awaitable co_spawn_cancellation_handler_bug() { auto strand = make_strand(executor); try { - co_await (sleep(1s) && spawn_throw_op(strand) && spawn_noop_loop(strand)); + co_await (my_sleep(1s) && spawn_throw_op(strand) && spawn_noop_loop(strand)); } catch (std::runtime_error&) { } } diff --git a/silkworm/node/bittorrent/client.cpp b/silkworm/node/bittorrent/client.cpp index 0823846fca..39984419d0 100644 --- a/silkworm/node/bittorrent/client.cpp +++ b/silkworm/node/bittorrent/client.cpp @@ -30,6 +30,7 @@ #include #include +#include #include #include #include @@ -294,10 +295,16 @@ bool BitTorrentClient::handle_alert(const lt::alert* alert) { // When we receive the finished alert, we request to save resume data for the torrent if (const auto* tfa = lt::alert_cast(alert)) { const auto& status = tfa->handle.status(); + std::tm completed_calendar_time{}; +#ifdef _MSC_VER + SILKWORM_ASSERT(gmtime_s(&completed_calendar_time, &status.completed_time) == 0); +#else + SILKWORM_ASSERT(gmtime_r(&status.completed_time, &completed_calendar_time) != nullptr); +#endif SILK_TRACE << "Torrent: " << tfa->torrent_name() << " finished download_rate: " << (status.download_rate / 1000) << " kB/s" << " download_payload_rate: " << (status.download_payload_rate / 1000) << " kB/s" << " in " << (status.completed_time - status.added_time) << " sec at " - << std::put_time(std::gmtime(&status.completed_time), "%c %Z"); + << std::put_time(&completed_calendar_time, "%c %Z"); tfa->handle.save_resume_data(lt::torrent_handle::save_info_dict | lt::torrent_handle::flush_disk_cache); ++outstanding_resume_requests_;