refactor: re-enable clang-tidy warnings (#2485)

libtransmission/.clang-tidy

@@ -13,25 +13,17 @@ Checks: >
   -modernize-avoid-c-arrays,
   -modernize-raw-string-literal,
   -modernize-redundant-void-arg,
-  -modernize-unary-static-assert,
-  -modernize-use-auto,
-  -modernize-use-bool-literals,
-  -modernize-use-nodiscard,
   -readability-braces-around-statements,
-  -readability-convert-member-functions-to-static,
   -readability-function-cognitive-complexity,
   -readability-implicit-bool-conversion,
   -readability-inconsistent-declaration-parameter-name,
-  -readability-isolate-declaration,
   -readability-magic-numbers,
   -readability-named-parameter,
   -readability-non-const-parameter,
   -readability-qualified-auto,
   -readability-redundant-access-specifiers,
-  -readability-simplify-boolean-expr,
   -readability-static-accessed-through-instance,
   -readability-static-definition-in-anonymous-namespace,
-  -readability-uppercase-literal-suffix,
   bugprone-argument-comment,
   bugprone-assert-side-effect,
   bugprone-bad-signal-to-kill-thread,
@@ -83,6 +75,7 @@ Checks: >
   bugprone-use-after-move,
   bugprone-virtual-near-miss,
   cppcoreguidelines-init-variables,
+  cppcoreguidelines-prefer-member-initializer,
   misc-static-assert,
   modernize-avoid-bind,
   modernize-concat-nested-namespaces,
@@ -97,11 +90,15 @@ Checks: >
   modernize-replace-random-shuffle,
   modernize-return-braced-init-list,
   modernize-shrink-to-fit,
+  modernize-unary-static-assert,
+  modernize-use-auto,
+  modernize-use-bool-literals,
   modernize-use-default-member-init,
   modernize-use-emplace,
   modernize-use-emplace,
   modernize-use-equals-default,
   modernize-use-equals-delete,
+  modernize-use-nodiscard,
   modernize-use-noexcept,
   modernize-use-nullptr,
   modernize-use-override,
@@ -112,11 +109,13 @@ Checks: >
   readability-avoid-const-params-in-decls,
   readability-const-return-type,
   readability-container-size-empty,
+  readability-convert-member-functions-to-static,
   readability-delete-null-pointer,
   readability-else-after-return,
   readability-function-size,
   readability-identifier-length,
   readability-identifier-naming,
+  readability-isolate-declaration,
   readability-make-member-function-const,
   readability-misleading-indentation,
   readability-misplaced-array-index,
@@ -128,7 +127,9 @@ Checks: >
   readability-redundant-smartptr-get,
   readability-redundant-string-cstr,
   readability-redundant-string-init,
+  readability-simplify-boolean-expr,
   readability-simplify-subscript-expr,
   readability-string-compare,
   readability-suspicious-call-argument,
-  readability-uniqueptr-delete-release
+  readability-uniqueptr-delete-release,
+  readability-uppercase-literal-suffix,

libtransmission/announce-list.h

@@ -111,7 +111,7 @@ private:
     tr_tracker_tier_t getTier(tr_tracker_tier_t tier, tr_url_parsed_t const& announce) const;
 
     bool canAdd(tr_url_parsed_t const& announce);
-    tr_tracker_id_t nextUniqueId();
+    static tr_tracker_id_t nextUniqueId();
     trackers_t::iterator find(std::string_view announce);
     trackers_t::iterator find(tr_tracker_id_t id);
 

libtransmission/announcer.cc

@@ -96,7 +96,7 @@ namespace
 
 struct StopsCompare
 {
-    int compare(tr_announce_request const* a, tr_announce_request const* b) const // <=>
+    static int compare(tr_announce_request const* a, tr_announce_request const* b) // <=>
     {
         // primary key: volume of data transferred
         auto const ax = a->up + a->down;
@@ -242,7 +242,7 @@ struct tr_tracker
     {
     }
 
-    int getRetryInterval() const
+    [[nodiscard]] int getRetryInterval() const
     {
         switch (consecutive_failures)
         {
@@ -529,12 +529,15 @@ struct tr_torrent_announcer
         return nullptr;
     }
 
-    bool canManualAnnounce() const
+    [[nodiscard]] bool canManualAnnounce() const
     {
         return std::any_of(std::begin(tiers), std::end(tiers), [](auto const& tier) { return tier.canManualAnnounce(); });
     }
 
-    bool findTracker(tr_interned_string const& announce_url, tr_tier const** setme_tier, tr_tracker const** setme_tracker) const
+    [[nodiscard]] bool findTracker(
+        tr_interned_string const& announce_url,
+        tr_tier const** setme_tier,
+        tr_tracker const** setme_tracker) const
     {
         for (auto const& tier : tiers)
         {

libtransmission/cache.cc

@@ -110,8 +110,8 @@ static int getBlockRun(tr_cache const* cache, int pos, struct run_info* info)
 /* higher rank comes before lower rank */
 static int compareRuns(void const* va, void const* vb)
 {
-    auto* a = static_cast<struct run_info const*>(va);
-    auto* b = static_cast<struct run_info const*>(vb);
+    auto const* const a = static_cast<struct run_info const*>(va);
+    auto const* const b = static_cast<struct run_info const*>(vb);
     return b->rank - a->rank;
 }
 

libtransmission/file-posix.cc

@@ -708,7 +708,7 @@ bool tr_sys_file_seek(tr_sys_file_t handle, int64_t offset, tr_seek_origin_t ori
     bool ret = false;
 
     off_t const my_new_offset = lseek(handle, offset, origin);
-    static_assert(sizeof(*new_offset) >= sizeof(my_new_offset), "");
+    static_assert(sizeof(*new_offset) >= sizeof(my_new_offset));
 
     if (my_new_offset != -1)
     {
@@ -735,7 +735,7 @@ bool tr_sys_file_read(tr_sys_file_t handle, void* buffer, uint64_t size, uint64_
     bool ret = false;
 
     ssize_t const my_bytes_read = read(handle, buffer, size);
-    static_assert(sizeof(*bytes_read) >= sizeof(my_bytes_read), "");
+    static_assert(sizeof(*bytes_read) >= sizeof(my_bytes_read));
 
     if (my_bytes_read != -1)
     {
@@ -779,7 +779,7 @@ bool tr_sys_file_read_at(
 
 #endif
 
-    static_assert(sizeof(*bytes_read) >= sizeof(my_bytes_read), "");
+    static_assert(sizeof(*bytes_read) >= sizeof(my_bytes_read));
 
     if (my_bytes_read != -1)
     {
@@ -806,7 +806,7 @@ bool tr_sys_file_write(tr_sys_file_t handle, void const* buffer, uint64_t size,
     bool ret = false;
 
     ssize_t const my_bytes_written = write(handle, buffer, size);
-    static_assert(sizeof(*bytes_written) >= sizeof(my_bytes_written), "");
+    static_assert(sizeof(*bytes_written) >= sizeof(my_bytes_written));
 
     if (my_bytes_written != -1)
     {
@@ -850,7 +850,7 @@ bool tr_sys_file_write_at(
 
 #endif
 
-    static_assert(sizeof(*bytes_written) >= sizeof(my_bytes_written), "");
+    static_assert(sizeof(*bytes_written) >= sizeof(my_bytes_written));
 
     if (my_bytes_written != -1)
     {

libtransmission/peer-mgr-active-requests.cc

@@ -32,7 +32,7 @@ struct peer_at
     {
     }
 
-    int compare(peer_at const& that) const // <=>
+    [[nodiscard]] int compare(peer_at const& that) const // <=>
     {
         if (peer != that.peer)
         {

libtransmission/peer-mgr-wishlist.cc

@@ -36,7 +36,7 @@ struct Candidate
     {
     }
 
-    int compare(Candidate const& that) const // <=>
+    [[nodiscard]] int compare(Candidate const& that) const // <=>
     {
         // prefer pieces closer to completion
         if (n_blocks_missing != that.n_blocks_missing)
@@ -129,7 +129,7 @@ std::vector<tr_block_span_t> makeSpans(tr_block_index_t const* sorted_blocks, si
 
 } // namespace
 
-std::vector<tr_block_span_t> Wishlist::next(Wishlist::PeerInfo const& peer_info, size_t n_wanted_blocks) const
+std::vector<tr_block_span_t> Wishlist::next(Wishlist::PeerInfo const& peer_info, size_t n_wanted_blocks)
 {
     size_t n_blocks = 0;
     auto spans = std::vector<tr_block_span_t>{};

libtransmission/peer-mgr-wishlist.h

@@ -36,5 +36,5 @@ public:
     };
 
     // get a list of the next blocks that we should request from a peer
-    std::vector<tr_block_span_t> next(PeerInfo const& peer_info, size_t n_wanted_blocks) const;
+    static std::vector<tr_block_span_t> next(PeerInfo const& peer_info, size_t n_wanted_blocks);
 };

libtransmission/peer-mgr.cc

@@ -189,7 +189,7 @@ public:
 
 struct tr_peerMgr
 {
-    auto unique_lock() const
+    [[nodiscard]] auto unique_lock() const
     {
         return session->unique_lock();
     }
@@ -549,7 +549,7 @@ static void updateEndgame(tr_swarm* s)
 
 std::vector<tr_block_span_t> tr_peerMgrGetNextRequests(tr_torrent* torrent, tr_peer const* peer, size_t numwant)
 {
-    class PeerInfoImpl : public Wishlist::PeerInfo
+    class PeerInfoImpl final : public Wishlist::PeerInfo
     {
     public:
         PeerInfoImpl(tr_torrent const* torrent_in, tr_peer const* peer_in)
@@ -559,44 +559,44 @@ std::vector<tr_block_span_t> tr_peerMgrGetNextRequests(tr_torrent* torrent, tr_p
         {
         }
 
-        ~PeerInfoImpl() override = default;
+        ~PeerInfoImpl() final = default;
 
-        bool clientCanRequestBlock(tr_block_index_t block) const override
+        [[nodiscard]] bool clientCanRequestBlock(tr_block_index_t block) const final
         {
             return !torrent_->hasBlock(block) && !swarm_->active_requests.has(block, peer_);
         }
 
-        bool clientCanRequestPiece(tr_piece_index_t piece) const override
+        [[nodiscard]] bool clientCanRequestPiece(tr_piece_index_t piece) const final
         {
             return torrent_->pieceIsWanted(piece) && peer_->have.test(piece);
         }
 
-        bool isEndgame() const override
+        [[nodiscard]] bool isEndgame() const final
         {
             return swarm_->endgame;
         }
 
-        size_t countActiveRequests(tr_block_index_t block) const override
+        [[nodiscard]] size_t countActiveRequests(tr_block_index_t block) const final
         {
             return swarm_->active_requests.count(block);
         }
 
-        size_t countMissingBlocks(tr_piece_index_t piece) const override
+        [[nodiscard]] size_t countMissingBlocks(tr_piece_index_t piece) const final
         {
             return torrent_->countMissingBlocksInPiece(piece);
         }
 
-        tr_block_span_t blockSpan(tr_piece_index_t piece) const override
+        [[nodiscard]] tr_block_span_t blockSpan(tr_piece_index_t piece) const final
         {
             return torrent_->blockSpanForPiece(piece);
         }
 
-        tr_piece_index_t countAllPieces() const override
+        [[nodiscard]] tr_piece_index_t countAllPieces() const final
         {
             return torrent_->pieceCount();
         }
 
-        tr_priority_t priority(tr_piece_index_t piece) const override
+        [[nodiscard]] tr_priority_t priority(tr_piece_index_t piece) const final
         {
             return torrent_->piecePriority(piece);
         }
@@ -1573,7 +1573,7 @@ void tr_peerUpdateProgress(tr_torrent* tor, tr_peer* peer)
 
     peer->progress = std::clamp(peer->progress, 0.0F, 1.0F);
 
-    if (peer->atom != nullptr && peer->progress >= 1.0f)
+    if (peer->atom != nullptr && peer->progress >= 1.0F)
     {
         atomSetSeed(tor->swarm, peer->atom);
     }
@@ -2513,7 +2513,7 @@ static void closeBadPeers(tr_swarm* s, time_t const now_sec)
 
 struct ComparePeerByActivity
 {
-    int compare(tr_peer const* a, tr_peer const* b) const // <=>
+    static int compare(tr_peer const* a, tr_peer const* b) // <=>
     {
         if (a->doPurge != b->doPurge)
         {

libtransmission/peer-msgs.cc

@@ -298,42 +298,42 @@ public:
         return Bps > 0;
     }
 
-    bool is_peer_choked() const override
+    [[nodiscard]] bool is_peer_choked() const override
     {
         return peer_is_choked_;
     }
 
-    bool is_peer_interested() const override
+    [[nodiscard]] bool is_peer_interested() const override
     {
         return peer_is_interested_;
     }
 
-    bool is_client_choked() const override
+    [[nodiscard]] bool is_client_choked() const override
     {
         return client_is_choked_;
     }
 
-    bool is_client_interested() const override
+    [[nodiscard]] bool is_client_interested() const override
     {
         return client_is_interested_;
     }
 
-    bool is_utp_connection() const override
+    [[nodiscard]] bool is_utp_connection() const override
     {
         return io->socket.type == TR_PEER_SOCKET_TYPE_UTP;
     }
 
-    bool is_encrypted() const override
+    [[nodiscard]] bool is_encrypted() const override
     {
         return tr_peerIoIsEncrypted(io);
     }
 
-    bool is_incoming_connection() const override
+    [[nodiscard]] bool is_incoming_connection() const override
     {
         return tr_peerIoIsIncoming(io);
     }
 
-    bool is_active(tr_direction direction) const override
+    [[nodiscard]] bool is_active(tr_direction direction) const override
     {
         TR_ASSERT(tr_isDirection(direction));
         auto const active = is_active_[direction];
@@ -348,12 +348,12 @@ public:
         set_active(direction, calculate_active(direction));
     }
 
-    time_t get_connection_age() const override
+    [[nodiscard]] time_t get_connection_age() const override
     {
         return tr_peerIoGetAge(io);
     }
 
-    bool is_reading_block(tr_block_index_t block) const override
+    [[nodiscard]] bool is_reading_block(tr_block_index_t block) const override
     {
         return state == AwaitingBtPiece && block == torrent->blockOf(incoming.blockReq.index, incoming.blockReq.offset);
     }
@@ -523,7 +523,7 @@ public:
     }
 
 private:
-    bool calculate_active(tr_direction direction) const
+    [[nodiscard]] bool calculate_active(tr_direction direction) const
     {
         if (direction == TR_CLIENT_TO_PEER)
         {

libtransmission/quark.cc

@@ -433,7 +433,8 @@ auto& my_runtime{ *new std::vector<std::string_view>{} };
 std::optional<tr_quark> tr_quark_lookup(std::string_view key)
 {
     // is it in our static array?
-    auto constexpr sbegin = std::begin(my_static), send = std::end(my_static);
+    auto constexpr sbegin = std::begin(my_static);
+    auto constexpr send = std::end(my_static);
     auto const sit = std::lower_bound(sbegin, send, key);
     if (sit != send && *sit == key)
     {
@@ -441,7 +442,8 @@ std::optional<tr_quark> tr_quark_lookup(std::string_view key)
     }
 
     /* was it added during runtime? */
-    auto const rbegin = std::begin(my_runtime), rend = std::end(my_runtime);
+    auto const rbegin = std::begin(my_runtime);
+    auto const rend = std::end(my_runtime);
     auto const rit = std::find(rbegin, rend, key);
     if (rit != rend)
     {

libtransmission/subprocess-posix.cc

@@ -87,7 +87,7 @@ static bool tr_spawn_async_in_parent(int pipe_fd, tr_error** error)
     int child_errno = 0;
     ssize_t count = 0;
 
-    static_assert(sizeof(child_errno) == sizeof(errno), "");
+    static_assert(sizeof(child_errno) == sizeof(errno));
 
     do
     {

libtransmission/torrent-metainfo.cc

@@ -572,7 +572,7 @@ bool tr_torrent_metainfo::migrateFile(
     std::string_view dirname,
     std::string_view name,
     std::string_view info_hash_string,
-    std::string_view suffix) const
+    std::string_view suffix)
 {
     auto const old_filename = makeFilename(dirname, name, info_hash_string, BasenameFormat::NameAndPartialHash, suffix);
     auto const old_filename_exists = tr_sys_path_exists(old_filename.c_str(), nullptr);

libtransmission/torrent-metainfo.h

@@ -152,13 +152,13 @@ public:
         return makeFilename(resume_dir, name(), infoHashString(), BasenameFormat::Hash, ".resume");
     }
 
-    bool migrateFile(
+    static bool migrateFile(
         std::string_view dirname,
         std::string_view name,
         std::string_view info_hash_string,
-        std::string_view suffix) const;
+        std::string_view suffix);
 
-    void removeFile(
+    static void removeFile(
         std::string_view dirname,
         std::string_view name,
         std::string_view info_hash_string,

libtransmission/tr-dht.cc

@@ -162,7 +162,7 @@ static void dht_bootstrap(void* closure)
             memcpy(&addr.addr.addr4, &cl->nodes[i * 6], 4);
             memcpy(&port, &cl->nodes[i * 6 + 4], 2);
             port = ntohs(port);
-            tr_dhtAddNode(cl->session, &addr, port, 1);
+            tr_dhtAddNode(cl->session, &addr, port, true);
         }
 
         if (i < num6 && !bootstrap_done(cl->session, AF_INET6))
@@ -175,7 +175,7 @@ static void dht_bootstrap(void* closure)
             memcpy(&addr.addr.addr6, &cl->nodes6[i * 18], 16);
             memcpy(&port, &cl->nodes6[i * 18 + 16], 2);
             port = ntohs(port);
-            tr_dhtAddNode(cl->session, &addr, port, 1);
+            tr_dhtAddNode(cl->session, &addr, port, true);
         }
 
         /* Our DHT code is able to take up to 9 nodes in a row without

libtransmission/utils.cc

@@ -500,7 +500,7 @@ extern "C"
 /* User-level routine. returns whether or not 'text' and 'p' matched */
 bool tr_wildmat(char const* text, char const* p)
 {
-    return (p[0] == '*' && p[1] == '\0') || (DoMatch(text, p) == true);
+    return (p[0] == '*' && p[1] == '\0') || (DoMatch(text, p) != 0);
 }
 
 char const* tr_strcasestr(char const* haystack, char const* needle)

libtransmission/variant.cc

@@ -868,7 +868,7 @@ public:
         }
     }
 
-    bool empty() const
+    [[nodiscard]] bool empty() const
     {
         return size == 0;
     }

libtransmission/verify.cc

@@ -163,7 +163,7 @@ struct verify_node
     void* callback_data;
     uint64_t current_size;
 
-    int compare(verify_node const& that) const
+    [[nodiscard]] int compare(verify_node const& that) const
     {
         // higher priority comes before lower priority
         auto const pa = tr_torrentGetPriority(torrent);

libtransmission/webseed.cc

@@ -65,6 +65,7 @@ public:
         , callback{ callback_in }
         , callback_data{ callback_data_in }
         , bandwidth(tor->bandwidth)
+        , timer(evtimer_new(session->event_base, webseed_timer_func, this))
     {
         // init parent bits
         have.setHasAll();
@@ -72,7 +73,6 @@ public:
 
         file_urls.resize(tor->fileCount());
 
-        timer = evtimer_new(session->event_base, webseed_timer_func, this);
         tr_timerAddMsec(timer, TR_IDLE_TIMER_MSEC);
     }
 
@@ -111,7 +111,7 @@ public:
 
     Bandwidth bandwidth;
     std::set<tr_webseed_task*> tasks;
-    struct event* timer = nullptr;
+    struct event* const timer;
     int consecutive_failures = 0;
     int retry_tickcount = 0;
     int retry_challenge = 0;