diff --git a/gtk/Application.cc b/gtk/Application.cc index a1b3499be..d4d76b080 100644 --- a/gtk/Application.cc +++ b/gtk/Application.cc @@ -630,13 +630,9 @@ namespace std::string get_application_id(std::string const& config_dir) { - struct stat sb; - ::stat(config_dir.c_str(), &sb); - - std::ostringstream id; - id << "com.transmissionbt.transmission_" << sb.st_dev << '_' << sb.st_ino; - - return id.str(); + struct stat sb = {}; + (void)::stat(config_dir.c_str(), &sb); + return fmt::format("com.transmissionbt.transmission_{}_{}", sb.st_dev, sb.st_ino); } } // namespace diff --git a/gtk/FaviconCache.cc b/gtk/FaviconCache.cc index d1e9bfc55..a614f7bb3 100644 --- a/gtk/FaviconCache.cc +++ b/gtk/FaviconCache.cc @@ -69,7 +69,7 @@ Glib::RefPtr favicon_load_from_cache(std::string const& host) } catch (Glib::Error const&) { - g_remove(filename.c_str()); + (void)g_remove(filename.c_str()); return {}; } } diff --git a/libtransmission/.clang-tidy b/libtransmission/.clang-tidy index 681531d4f..30453ab82 100644 --- a/libtransmission/.clang-tidy +++ b/libtransmission/.clang-tidy @@ -20,6 +20,7 @@ Checks: > cppcoreguidelines-slicing, cppcoreguidelines-special-member-functions, cppcoreguidelines-virtual-class-destructor, + google-explicit-constructor, misc-*, -misc-no-recursion, -misc-non-private-member-variables-in-classes, diff --git a/libtransmission/bandwidth.cc b/libtransmission/bandwidth.cc index 24e4a0a96..0b93332d2 100644 --- a/libtransmission/bandwidth.cc +++ b/libtransmission/bandwidth.cc @@ -90,18 +90,15 @@ tr_bandwidth::tr_bandwidth(tr_bandwidth* parent) **** ***/ -static void remove_child(std::vector& v, tr_bandwidth* remove_me) +static void remove_child(std::vector& v, tr_bandwidth* remove_me) noexcept { - auto it = std::find(std::begin(v), std::end(v), remove_me); - if (it == std::end(v)) - { - return; - } - // the list isn't sorted -- so instead of erase()ing `it`, // do the cheaper option of overwriting it with the final item - *it = v.back(); - v.resize(v.size() - 1); + if (auto it = std::find(std::begin(v), std::end(v), remove_me); it != std::end(v)) + { + *it = v.back(); + v.resize(v.size() - 1); + } } void tr_bandwidth::setParent(tr_bandwidth* new_parent) diff --git a/libtransmission/file-posix.cc b/libtransmission/file-posix.cc index 252478cef..688ce5756 100644 --- a/libtransmission/file-posix.cc +++ b/libtransmission/file-posix.cc @@ -358,7 +358,7 @@ std::string tr_sys_path_resolve(std::string_view path, tr_error** error) auto const szpath = tr_pathbuf{ path }; auto buf = std::array{}; - if (auto* const ret = realpath(szpath, std::data(buf)); ret != nullptr) + if (auto const* const ret = realpath(szpath, std::data(buf)); ret != nullptr) { return ret; } @@ -1192,7 +1192,7 @@ std::string tr_sys_dir_get_current(tr_error** error) for (;;) { - if (char* const ret = getcwd(std::data(buf), std::size(buf)); ret != nullptr) + if (char const* const ret = getcwd(std::data(buf), std::size(buf)); ret != nullptr) { return ret; } diff --git a/libtransmission/handshake.cc b/libtransmission/handshake.cc index 142eb7481..812b5dfd4 100644 --- a/libtransmission/handshake.cc +++ b/libtransmission/handshake.cc @@ -113,9 +113,10 @@ enum handshake_state_t struct tr_handshake { - tr_handshake(std::shared_ptr mediator_in) + tr_handshake(std::shared_ptr mediator_in, tr_encryption_mode encryption_mode_in) : mediator{ std::move(mediator_in) } , dh{ mediator->privateKey() } + , encryption_mode{ encryption_mode_in } { } @@ -143,7 +144,7 @@ struct tr_handshake bool haveSentBitTorrentHandshake = false; tr_peerIo* io = nullptr; DH dh = {}; - handshake_state_t state; + handshake_state_t state = AWAITING_HANDSHAKE; tr_encryption_mode encryption_mode; uint16_t pad_c_len = {}; uint16_t pad_d_len = {}; @@ -1137,9 +1138,8 @@ tr_handshake* tr_handshakeNew( tr_handshake_done_func done_func, void* done_func_user_data) { - auto* const handshake = new tr_handshake{ std::move(mediator) }; + auto* const handshake = new tr_handshake{ std::move(mediator), encryption_mode }; handshake->io = io; - handshake->encryption_mode = encryption_mode; handshake->done_func = done_func; handshake->done_func_user_data = done_func_user_data; handshake->timeout_timer = handshake->mediator->createTimer(); diff --git a/libtransmission/makemeta.cc b/libtransmission/makemeta.cc index 7410ff54c..5c07927ba 100644 --- a/libtransmission/makemeta.cc +++ b/libtransmission/makemeta.cc @@ -66,7 +66,7 @@ void walkTree(std::string_view const top, std::string_view const subpath, std::s tr_sys_path_native_separators(std::data(path)); tr_error* error = nullptr; auto const info = tr_sys_path_get_info(path, 0, &error); - if (!info) + if (error != nullptr) { tr_logAddWarn(fmt::format( _("Skipping '{path}': {error} ({error_code})"), @@ -74,6 +74,9 @@ void walkTree(std::string_view const top, std::string_view const subpath, std::s fmt::arg("error", error->message), fmt::arg("error_code", error->code))); tr_error_free(error); + } + if (!info) + { return; } diff --git a/libtransmission/natpmp_local.h b/libtransmission/natpmp_local.h index 357876fb6..7a29def27 100644 --- a/libtransmission/natpmp_local.h +++ b/libtransmission/natpmp_local.h @@ -56,7 +56,7 @@ private: void setCommandTime(); - natpmp_t natpmp_; + natpmp_t natpmp_ = {}; tr_port public_port_ = {}; tr_port private_port_ = {}; diff --git a/libtransmission/peer-mgr.cc b/libtransmission/peer-mgr.cc index 992117ef5..878ae7e8f 100644 --- a/libtransmission/peer-mgr.cc +++ b/libtransmission/peer-mgr.cc @@ -84,7 +84,7 @@ private: } public: - tr_handshake_mediator_impl(tr_session& session) + explicit tr_handshake_mediator_impl(tr_session& session) : session_{ session } { } diff --git a/libtransmission/port-forwarding.cc b/libtransmission/port-forwarding.cc index 00412e7de..aab627034 100644 --- a/libtransmission/port-forwarding.cc +++ b/libtransmission/port-forwarding.cc @@ -24,7 +24,7 @@ using namespace std::literals; struct tr_shared { - tr_shared(tr_session& session_in) + explicit tr_shared(tr_session& session_in) : session{ session_in } { } diff --git a/libtransmission/rpc-server.cc b/libtransmission/rpc-server.cc index aef465e57..1044c48b0 100644 --- a/libtransmission/rpc-server.cc +++ b/libtransmission/rpc-server.cc @@ -109,7 +109,7 @@ static bool constexpr tr_rpc_address_is_valid(tr_rpc_address const& a) **** ***/ -static char const* get_current_session_id(tr_rpc_server* server) +static char const* get_current_session_id(tr_rpc_server const* server) { return server->session->session_id.c_str(); } diff --git a/libtransmission/session-id.h b/libtransmission/session-id.h index f7e8edab0..e6632d9a5 100644 --- a/libtransmission/session-id.h +++ b/libtransmission/session-id.h @@ -17,7 +17,7 @@ class tr_session_id public: using current_time_func_t = time_t (*)(); - tr_session_id(current_time_func_t get_current_time) + explicit tr_session_id(current_time_func_t get_current_time) : get_current_time_{ get_current_time } { } @@ -46,7 +46,7 @@ private: static auto constexpr SessionIdSize = size_t{ 48 }; static auto constexpr SessionIdDurationSec = time_t{ 60 * 60 }; /* expire in an hour */ - using session_id_t = std::array; // +1 for '\0'; + using session_id_t = std::array; // add one for '\0' static session_id_t make_session_id(); current_time_func_t const get_current_time_; diff --git a/libtransmission/session.h b/libtransmission/session.h index e491b2eb1..79213d77b 100644 --- a/libtransmission/session.h +++ b/libtransmission/session.h @@ -86,7 +86,7 @@ struct tr_bindinfo struct tr_turtle_info { /* TR_UP and TR_DOWN speed limits */ - unsigned int speedLimit_Bps[2] = {}; + std::array speedLimit_Bps = {}; /* is turtle mode on right now? */ bool isEnabled = false; diff --git a/libtransmission/timer-ev.cc b/libtransmission/timer-ev.cc index 402da66d0..016392f7d 100644 --- a/libtransmission/timer-ev.cc +++ b/libtransmission/timer-ev.cc @@ -19,7 +19,7 @@ namespace libtransmission class EvTimer final : public Timer { public: - EvTimer(struct event_base* base) + explicit EvTimer(struct event_base* base) : base_{ base } { setRepeating(is_repeating_); @@ -106,7 +106,7 @@ private: static_cast(vself)->handleTimer(); } - void handleTimer() + void handleTimer() const { TR_ASSERT(callback_); callback_(); diff --git a/libtransmission/utils.cc b/libtransmission/utils.cc index c342c2cc1..b82aa7084 100644 --- a/libtransmission/utils.cc +++ b/libtransmission/utils.cc @@ -103,7 +103,7 @@ bool tr_loadFile(std::string_view filename, std::vector& contents, tr_erro /* try to stat the file */ tr_error* my_error = nullptr; auto const info = tr_sys_path_get_info(szfilename, 0, &my_error); - if (!info) + if (my_error != nullptr) { tr_logAddError(fmt::format( _("Couldn't read '{path}': {error} ({error_code})"), @@ -114,7 +114,7 @@ bool tr_loadFile(std::string_view filename, std::vector& contents, tr_erro return false; } - if (!info->isFile()) + if (!info || !info->isFile()) { tr_logAddError(fmt::format(_("Couldn't read '{path}': Not a regular file"), fmt::arg("path", filename))); tr_error_set(error, TR_ERROR_EISDIR, "Not a regular file"sv); diff --git a/tests/libtransmission/announce-list-test.cc b/tests/libtransmission/announce-list-test.cc index b95405146..b9bbba022 100644 --- a/tests/libtransmission/announce-list-test.cc +++ b/tests/libtransmission/announce-list-test.cc @@ -385,7 +385,7 @@ TEST_F(AnnounceListTest, save) std::end(modified_tm.announceList()))); // cleanup - std::remove(test_file.c_str()); + (void)std::remove(test_file.c_str()); } TEST_F(AnnounceListTest, SingleAnnounce) diff --git a/tests/libtransmission/test-fixtures.h b/tests/libtransmission/test-fixtures.h index 4b6437923..e312b5325 100644 --- a/tests/libtransmission/test-fixtures.h +++ b/tests/libtransmission/test-fixtures.h @@ -127,15 +127,13 @@ protected: } tr_error* error = nullptr; - - if (auto path = tr_sys_dir_get_current(&error); !std::empty(path)) + auto path = tr_sys_dir_get_current(&error); + if (error != nullptr) { - return path; + std::cerr << "tr_sys_dir_get_current error: '" << error->message << "'" << std::endl; + tr_error_free(error); } - - std::cerr << "tr_sys_dir_get_current error: '" << error->message << "'" << std::endl; - tr_error_free(error); - return {}; + return path; } static std::string create_sandbox(std::string const& parent_dir, std::string const& tmpl) @@ -205,7 +203,7 @@ protected: { uint64_t n = {}; tr_error* local_error = nullptr; - if (!tr_sys_file_write(fd, left, n_left, &n, error)) + if (!tr_sys_file_write(fd, left, n_left, &n, &local_error)) { fprintf(stderr, "Error writing file: '%s'\n", local_error->message); tr_error_propagate(error, &local_error); diff --git a/tests/libtransmission/watchdir-test.cc b/tests/libtransmission/watchdir-test.cc index bcd4b7a06..2adce185f 100644 --- a/tests/libtransmission/watchdir-test.cc +++ b/tests/libtransmission/watchdir-test.cc @@ -78,7 +78,12 @@ protected: auto watchdir = force_generic ? Watchdir::createGeneric(path, std::move(callback), *timer_maker_, GenericRescanInterval) : Watchdir::create(path, std::move(callback), *timer_maker_, ev_base_.get()); - dynamic_cast(watchdir.get())->setRetryDuration(RetryDuration); + + if (auto* const base_watchdir = dynamic_cast(watchdir.get()); base_watchdir != nullptr) + { + base_watchdir->setRetryDuration(RetryDuration); + } + return watchdir; } @@ -230,7 +235,9 @@ TEST_P(WatchDirTest, retry) auto watchdir = createWatchDir(path, callback); auto constexpr FastRetryWaitTime = 20ms; auto constexpr ThreeRetries = FastRetryWaitTime * 4; - dynamic_cast(watchdir.get())->setRetryDuration(FastRetryWaitTime); + auto* const base_watchdir = dynamic_cast(watchdir.get()); + ASSERT_TRUE(base_watchdir != nullptr); + base_watchdir->setRetryDuration(FastRetryWaitTime); processEvents(ThreeRetries); EXPECT_EQ(0U, std::size(names)); diff --git a/utils/create.cc b/utils/create.cc index f398d73f8..4c8e6c4da 100644 --- a/utils/create.cc +++ b/utils/create.cc @@ -129,14 +129,12 @@ int parseCommandLine(app_options& options, int argc, char const* const* argv) std::string tr_getcwd() { tr_error* error = nullptr; - if (auto cur = tr_sys_dir_get_current(&error); !std::empty(cur)) + auto cur = tr_sys_dir_get_current(&error); { - return cur; + fprintf(stderr, "getcwd error: \"%s\"", error->message); + tr_error_free(error); } - - fprintf(stderr, "getcwd error: \"%s\"", error->message); - tr_error_free(error); - return ""; + return cur; } } // namespace