From 76d854dcc89f1fec252e678dc09ec73c4d776f24 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Mon, 10 Mar 2025 15:01:31 -0500 Subject: [PATCH] fix: clang-tidy-20 warnings (#7479) * chore: disable unavoidable warning * fix: clang-tidy readability-math-missing-parentheses warnings * fix: clang-tidy google-readability-todo warnings * fix: clang-tidy misc-use-internal-linkage warnings * fix: clang-tidy readability-redundant-string-cstr warnings * chore: disable cppcoreguidelines-avoid-const-or-ref-data-members warnings in tests * chore: disable cppcoreguidelines-avoid-const-or-ref-data-members warnings in qt/ * fix: clang-tidy readability-identifier-naming warnings --- libtransmission/file-posix.cc | 2 +- qt/.clang-tidy | 1 + qt/Application.cc | 5 ++--- tests/libtransmission/.clang-tidy | 1 + tests/libtransmission/block-info-test.cc | 14 +++++++------- tests/libtransmission/completion-test.cc | 10 +++++----- tests/libtransmission/dht-test.cc | 6 +++--- tests/libtransmission/file-piece-map-test.cc | 2 +- tests/libtransmission/ip-cache-test.cc | 2 +- tests/libtransmission/session-test.cc | 3 ++- tests/libtransmission/subprocess-test.cc | 9 +++++++-- tests/libtransmission/timer-test.cc | 8 ++++---- 12 files changed, 35 insertions(+), 28 deletions(-) diff --git a/libtransmission/file-posix.cc b/libtransmission/file-posix.cc index 5919f1343..31a6c8172 100644 --- a/libtransmission/file-posix.cc +++ b/libtransmission/file-posix.cc @@ -569,7 +569,7 @@ tr_sys_file_t tr_sys_file_open(char const* path, int flags, int permissions, tr_ { TR_SYS_FILE_SEQUENTIAL, TR_SYS_FILE_SEQUENTIAL, O_SEQUENTIAL } } }; - int native_flags = O_BINARY | O_LARGEFILE | O_CLOEXEC; + int native_flags = O_BINARY | O_LARGEFILE | O_CLOEXEC; // NOLINT(misc-redundant-expression) for (auto const& item : NativeMap) { diff --git a/qt/.clang-tidy b/qt/.clang-tidy index 05053fcf2..2cb37213e 100644 --- a/qt/.clang-tidy +++ b/qt/.clang-tidy @@ -9,6 +9,7 @@ Checks: > cert-*, clang-analyzer-*, cppcoreguidelines-*, + -cppcoreguidelines-avoid-const-or-ref-data-members, -cppcoreguidelines-avoid-magic-numbers, -cppcoreguidelines-macro-usage, -cppcoreguidelines-narrowing-conversions, diff --git a/qt/Application.cc b/qt/Application.cc index b8846ed75..c5a9c43c3 100644 --- a/qt/Application.cc +++ b/qt/Application.cc @@ -551,10 +551,9 @@ bool Application::notifyApp(QString const& title, QString const& body, QStringLi #ifdef QT_DBUS_LIB void Application::onNotificationActionInvoked(quint32 /* notification_id */, QString action_key) { - static QRegularExpression const start_now_regex{ QStringLiteral(R"rgx(start-now\((\d+)\))rgx") }; + static QRegularExpression const StartNowRegex{ QStringLiteral(R"rgx(start-now\((\d+)\))rgx") }; - auto const match = start_now_regex.match(action_key); - if (match.hasMatch()) + if (auto const match = StartNowRegex.match(action_key); match.hasMatch()) { int const torrent_id = match.captured(1).toInt(); session_->startTorrentsNow({ torrent_id }); diff --git a/tests/libtransmission/.clang-tidy b/tests/libtransmission/.clang-tidy index ac0d561e1..470c7dc0c 100644 --- a/tests/libtransmission/.clang-tidy +++ b/tests/libtransmission/.clang-tidy @@ -9,6 +9,7 @@ Checks: > cert-*, clang-analyzer-optin*, cppcoreguidelines-*, + -cppcoreguidelines-avoid-const-or-ref-data-members, -cppcoreguidelines-avoid-magic-numbers, -cppcoreguidelines-avoid-non-const-global-variables, -cppcoreguidelines-macro-usage, diff --git a/tests/libtransmission/block-info-test.cc b/tests/libtransmission/block-info-test.cc index 7d2feb1ba..59a014426 100644 --- a/tests/libtransmission/block-info-test.cc +++ b/tests/libtransmission/block-info-test.cc @@ -43,7 +43,7 @@ TEST_F(BlockInfoTest, handlesOddSize) static auto constexpr ExpectedBlocksPerPiece = uint64_t{ 4U }; static auto constexpr PieceSize = ExpectedBlockSize * ExpectedBlocksPerPiece; static auto constexpr PieceCount = uint64_t{ 5U }; - static auto constexpr TotalSize = PieceSize * (PieceCount - 1U) + 1U; + static auto constexpr TotalSize = (PieceSize * (PieceCount - 1U)) + 1U; auto const info = tr_block_info{ TotalSize, PieceSize }; @@ -60,7 +60,7 @@ TEST_F(BlockInfoTest, pieceSize) static auto constexpr ExpectedBlocksPerPiece = uint64_t{ 4U }; static auto constexpr PieceSize = ExpectedBlockSize * ExpectedBlocksPerPiece; static auto constexpr PieceCount = uint64_t{ 5U }; - static auto constexpr TotalSize = PieceSize * (PieceCount - 1U) + 1U; + static auto constexpr TotalSize = (PieceSize * (PieceCount - 1U)) + 1U; auto const info = tr_block_info{ TotalSize, PieceSize }; @@ -74,7 +74,7 @@ TEST_F(BlockInfoTest, blockSize) static auto constexpr ExpectedBlocksPerPiece = uint64_t{ 4U }; static auto constexpr PieceSize = ExpectedBlockSize * ExpectedBlocksPerPiece; static auto constexpr PieceCount = uint64_t{ 5U }; - static auto constexpr TotalSize = PieceSize * (PieceCount - 1) + 1; + static auto constexpr TotalSize = (PieceSize * (PieceCount - 1)) + 1; auto const info = tr_block_info{ TotalSize, PieceSize }; @@ -88,7 +88,7 @@ TEST_F(BlockInfoTest, blockSpanForPiece) static auto constexpr ExpectedBlocksPerPiece = uint64_t{ 4U }; static auto constexpr PieceSize = ExpectedBlockSize * ExpectedBlocksPerPiece; static auto constexpr PieceCount = uint64_t{ 5U }; - static auto constexpr TotalSize = PieceSize * (PieceCount - 1U) + 1U; + static auto constexpr TotalSize = (PieceSize * (PieceCount - 1U)) + 1U; auto info = tr_block_info{ TotalSize, PieceSize }; @@ -111,7 +111,7 @@ TEST_F(BlockInfoTest, blockLoc) static auto constexpr ExpectedBlocksPerPiece = uint64_t{ 4U }; static auto constexpr PieceSize = ExpectedBlockSize * ExpectedBlocksPerPiece; static auto constexpr PieceCount = uint64_t{ 5U }; - static auto constexpr TotalSize = PieceSize * (PieceCount - 1U) + 1U; + static auto constexpr TotalSize = (PieceSize * (PieceCount - 1U)) + 1U; auto const info = tr_block_info{ TotalSize, PieceSize }; @@ -142,7 +142,7 @@ TEST_F(BlockInfoTest, pieceLoc) static auto constexpr ExpectedBlocksPerPiece = uint64_t{ 4U }; static auto constexpr PieceSize = ExpectedBlockSize * ExpectedBlocksPerPiece; static auto constexpr PieceCount = uint64_t{ 5U }; - static auto constexpr TotalSize = PieceSize * (PieceCount - 1U) + 1U; + static auto constexpr TotalSize = (PieceSize * (PieceCount - 1U)) + 1U; auto const info = tr_block_info{ TotalSize, PieceSize }; @@ -189,7 +189,7 @@ TEST_F(BlockInfoTest, byteLoc) static auto constexpr ExpectedBlocksPerPiece = uint64_t{ 4U }; static auto constexpr PieceSize = ExpectedBlockSize * ExpectedBlocksPerPiece; static auto constexpr PieceCount = uint64_t{ 5U }; - static auto constexpr TotalSize = PieceSize * (PieceCount - 1U) + 1U; + static auto constexpr TotalSize = (PieceSize * (PieceCount - 1U)) + 1U; auto const info = tr_block_info{ TotalSize, PieceSize }; diff --git a/tests/libtransmission/completion-test.cc b/tests/libtransmission/completion-test.cc index 2f53bee80..82f572be4 100644 --- a/tests/libtransmission/completion-test.cc +++ b/tests/libtransmission/completion-test.cc @@ -280,7 +280,7 @@ TEST_F(CompletionTest, leftUntilDone) // check that dnd-flagging a piece we DON'T already have adjusts by block_info.pieceSize() torrent.dnd_pieces.insert(1); completion.invalidate_size_when_done(); - EXPECT_EQ(block_info.total_size() - block_info.piece_size() * uint64_t{ 2U }, completion.left_until_done()); + EXPECT_EQ(block_info.total_size() - (block_info.piece_size() * uint64_t{ 2U }), completion.left_until_done()); torrent.dnd_pieces.clear(); completion.invalidate_size_when_done(); @@ -325,7 +325,7 @@ TEST_F(CompletionTest, sizeWhenDone) torrent.dnd_pieces.insert(i); } completion.invalidate_size_when_done(); - EXPECT_EQ(block_info.total_size() - uint64_t{ 16U } * block_info.piece_size(), completion.size_when_done()); + EXPECT_EQ(block_info.total_size() - (uint64_t{ 16U } * block_info.piece_size()), completion.size_when_done()); } TEST_F(CompletionTest, createPieceBitfield) @@ -458,19 +458,19 @@ TEST_F(CompletionTest, countHasBytesInSpan) // test span that has a middle block EXPECT_EQ(BlockSize * 3, completion.count_has_bytes_in_span({ 0, BlockSize * 3 })); - EXPECT_EQ(BlockSize * 2, completion.count_has_bytes_in_span({ BlockSize / 2, BlockSize * 2 + BlockSize / 2 })); + EXPECT_EQ(BlockSize * 2, completion.count_has_bytes_in_span({ BlockSize / 2, (BlockSize * 2) + (BlockSize / 2) })); // test span where first block is missing blocks.unset(0); completion.set_blocks(blocks); EXPECT_EQ(BlockSize * 2, completion.count_has_bytes_in_span({ 0, BlockSize * 3 })); - EXPECT_EQ(BlockSize * 1.5, completion.count_has_bytes_in_span({ BlockSize / 2, BlockSize * 2 + BlockSize / 2 })); + EXPECT_EQ(BlockSize * 1.5, completion.count_has_bytes_in_span({ BlockSize / 2, (BlockSize * 2) + (BlockSize / 2) })); // test span where final block is missing blocks.set_has_all(); blocks.unset(2); completion.set_blocks(blocks); EXPECT_EQ(BlockSize * 2, completion.count_has_bytes_in_span({ 0, BlockSize * 3 })); - EXPECT_EQ(BlockSize * 1.5, completion.count_has_bytes_in_span({ BlockSize / 2, BlockSize * 2 + BlockSize / 2 })); + EXPECT_EQ(BlockSize * 1.5, completion.count_has_bytes_in_span({ BlockSize / 2, (BlockSize * 2) + (BlockSize / 2) })); } TEST_F(CompletionTest, wantNone) diff --git a/tests/libtransmission/dht-test.cc b/tests/libtransmission/dht-test.cc index 62a96d0f2..ef41e51d5 100644 --- a/tests/libtransmission/dht-test.cc +++ b/tests/libtransmission/dht-test.cc @@ -60,17 +60,17 @@ using namespace std::literals; namespace libtransmission::test { +namespace +{ bool waitFor(struct event_base* event_base, std::chrono::milliseconds msec) { - return waitFor( // + return libtransmission::test::waitFor( // event_base, []() { return false; }, msec); } -namespace -{ auto constexpr IdLength = size_t{ 20U }; auto constexpr MockTimerInterval = 40ms; diff --git a/tests/libtransmission/file-piece-map-test.cc b/tests/libtransmission/file-piece-map-test.cc index 8466fd62c..09bae23b0 100644 --- a/tests/libtransmission/file-piece-map-test.cc +++ b/tests/libtransmission/file-piece-map-test.cc @@ -38,7 +38,7 @@ protected: 8U, 7U, 6U, - (3U * PieceSize + PieceSize / 2U - 10U - 9U - 8U - 7U - 6U), // [offset 5.75P +10+9+8+7+6] ends end-of-torrent + ((3U * PieceSize) + (PieceSize / 2U) - 10U - 9U - 8U - 7U - 6U), // [offset 5.75P +10+9+8+7+6] ends end-of-torrent 0U, // [offset 10P+1] zero-sized files at the end-of-torrent 0U, 0U, diff --git a/tests/libtransmission/ip-cache-test.cc b/tests/libtransmission/ip-cache-test.cc index b8ad7ecd0..a019655c4 100644 --- a/tests/libtransmission/ip-cache-test.cc +++ b/tests/libtransmission/ip-cache-test.cc @@ -230,7 +230,7 @@ TEST_F(IPCacheTest, onResponseIPQuery) struct LocalMockMediator final : public MockMediator { - void fetch(tr_web::FetchOptions&& options) override + void fetch(tr_web::FetchOptions&& options) override // NOLINT(cppcoreguidelines-rvalue-reference-param-not-moved) { auto response = tr_web::FetchResponse{ http_code, std::string{ AddrStr[k_] }, std::string{}, true, false, options.done_func_user_data }; diff --git a/tests/libtransmission/session-test.cc b/tests/libtransmission/session-test.cc index 82f29bb61..d73990932 100644 --- a/tests/libtransmission/session-test.cc +++ b/tests/libtransmission/session-test.cc @@ -208,8 +208,8 @@ namespace current_time_mock { namespace { + auto value = time_t{}; -} time_t get() { @@ -221,6 +221,7 @@ void set(time_t now) value = now; } +} // unnamed namespace } // namespace current_time_mock TEST_F(SessionTest, sessionId) diff --git a/tests/libtransmission/subprocess-test.cc b/tests/libtransmission/subprocess-test.cc index f2c95ce63..433dde916 100644 --- a/tests/libtransmission/subprocess-test.cc +++ b/tests/libtransmission/subprocess-test.cc @@ -29,13 +29,18 @@ namespace libtransmission::test { -std::string getTestProgramPath(std::string const& filename) +namespace { - auto const exe_path = tr_sys_path_resolve(testing::internal::GetArgvs().front().data()); + +std::string getTestProgramPath(std::string_view const filename) +{ + auto const exe_path = tr_sys_path_resolve(testing::internal::GetArgvs().front()); auto const exe_dir = tr_sys_path_dirname(exe_path); return fmt::format("{:s}/{:s}", exe_dir, filename); } +} // unnamed namespace + class SubprocessTest : public ::testing::Test , public testing::WithParamInterface diff --git a/tests/libtransmission/timer-test.cc b/tests/libtransmission/timer-test.cc index 7a81ec93e..49d495461 100644 --- a/tests/libtransmission/timer-test.cc +++ b/tests/libtransmission/timer-test.cc @@ -177,7 +177,7 @@ TEST_F(TimerTest, repeatingHonorsInterval) EXPECT_EQ(DesiredLoops, n_calls); } -// TODO: flaky test should be fixed instead of disabled +// TODO(ckerr): flaky test should be fixed instead of disabled TEST_F(TimerTest, DISABLED_restartWithDifferentInterval) { auto timer_maker = EvTimerMaker{ evbase_.get() }; @@ -207,7 +207,7 @@ TEST_F(TimerTest, DISABLED_restartWithDifferentInterval) test(200ms); } -// TODO: flaky test should be fixed instead of disabled +// TODO(ckerr): flaky test should be fixed instead of disabled TEST_F(TimerTest, DISABLED_restartWithSameInterval) { auto timer_maker = EvTimerMaker{ evbase_.get() }; @@ -237,7 +237,7 @@ TEST_F(TimerTest, DISABLED_restartWithSameInterval) test(timer->interval()); } -// TODO: flaky test should be fixed instead of disabled +// TODO(ckerr): flaky test should be fixed instead of disabled TEST_F(TimerTest, DISABLED_repeatingThenSingleShot) { auto timer_maker = EvTimerMaker{ evbase_.get() }; @@ -280,7 +280,7 @@ TEST_F(TimerTest, DISABLED_repeatingThenSingleShot) EXPECT_EQ(baseline + 1, n_calls); } -// TODO: flaky test should be fixed instead of disabled +// TODO(ckerr): flaky test should be fixed instead of disabled TEST_F(TimerTest, DISABLED_singleShotStop) { auto timer_maker = EvTimerMaker{ evbase_.get() };