fix: clang warnings (#2339)

* fix: comparison-is-always-false warning

* fix: C/C++ mixing (memset() to clear a C++ object)

libtransmission/crypto.cc

@@ -52,24 +52,21 @@ static void ensureKeyExists(tr_crypto* crypto)
     }
 }
 
-void tr_cryptoConstruct(tr_crypto* crypto, tr_sha1_digest_t const* torrent_hash, bool isIncoming)
+tr_crypto::tr_crypto(tr_sha1_digest_t const* torrent_hash_in, bool is_incoming_in)
+    : is_incoming{ is_incoming_in }
 {
-    memset(crypto, 0, sizeof(tr_crypto));
-
-    crypto->isIncoming = isIncoming;
-
-    if (torrent_hash != nullptr)
+    if (torrent_hash_in != nullptr)
     {
-        tr_cryptoSetTorrentHash(crypto, *torrent_hash);
+        this->torrent_hash = *torrent_hash_in;
     }
 }
 
-void tr_cryptoDestruct(tr_crypto* crypto)
+tr_crypto::~tr_crypto()
 {
-    tr_dh_secret_free(crypto->mySecret);
-    tr_dh_free(crypto->dh);
-    tr_free(crypto->enc_key);
-    tr_free(crypto->dec_key);
+    tr_dh_secret_free(this->mySecret);
+    tr_dh_free(this->dh);
+    tr_free(this->enc_key);
+    tr_free(this->dec_key);
 }
 
 /**
@@ -128,7 +125,7 @@ static void crypt_rc4(struct arc4_context* key, size_t buf_len, void const* buf_
 
 void tr_cryptoDecryptInit(tr_crypto* crypto)
 {
-    init_rc4(crypto, &crypto->dec_key, crypto->isIncoming ? "keyA" : "keyB"); // lgtm[cpp/weak-cryptographic-algorithm]
+    init_rc4(crypto, &crypto->dec_key, crypto->is_incoming ? "keyA" : "keyB"); // lgtm[cpp/weak-cryptographic-algorithm]
 }
 
 void tr_cryptoDecrypt(tr_crypto* crypto, size_t buf_len, void const* buf_in, void* buf_out)
@@ -138,7 +135,7 @@ void tr_cryptoDecrypt(tr_crypto* crypto, size_t buf_len, void const* buf_in, voi
 
 void tr_cryptoEncryptInit(tr_crypto* crypto)
 {
-    init_rc4(crypto, &crypto->enc_key, crypto->isIncoming ? "keyB" : "keyA"); // lgtm[cpp/weak-cryptographic-algorithm]
+    init_rc4(crypto, &crypto->enc_key, crypto->is_incoming ? "keyB" : "keyA"); // lgtm[cpp/weak-cryptographic-algorithm]
 }
 
 void tr_cryptoEncrypt(tr_crypto* crypto, size_t buf_len, void const* buf_in, void* buf_out)

libtransmission/crypto.h

@@ -35,21 +35,18 @@ enum
 /** @brief Holds state information for encrypted peer communications */
 struct tr_crypto
 {
-    std::optional<tr_sha1_digest_t> torrent_hash;
-    struct arc4_context* dec_key;
-    struct arc4_context* enc_key;
-    tr_dh_ctx_t dh;
-    uint8_t myPublicKey[KEY_LEN];
-    tr_dh_secret_t mySecret;
-    bool isIncoming;
+    tr_crypto(tr_sha1_digest_t const* torrent_hash = nullptr, bool is_incoming = true);
+    ~tr_crypto();
+
+    std::optional<tr_sha1_digest_t> torrent_hash = {};
+    struct arc4_context* dec_key = nullptr;
+    struct arc4_context* enc_key = nullptr;
+    tr_dh_ctx_t dh = {};
+    uint8_t myPublicKey[KEY_LEN] = {};
+    tr_dh_secret_t mySecret = {};
+    bool is_incoming = false;
 };
 
-/** @brief construct a new tr_crypto object */
-void tr_cryptoConstruct(tr_crypto* crypto, tr_sha1_digest_t const* torrent_hash, bool isIncoming);
-
-/** @brief destruct an existing tr_crypto object */
-void tr_cryptoDestruct(tr_crypto* crypto);
-
 void tr_cryptoSetTorrentHash(tr_crypto* crypto, tr_sha1_digest_t const& torrent_hash);
 
 std::optional<tr_sha1_digest_t> tr_cryptoGetTorrentHash(tr_crypto const* crypto);

libtransmission/interned-string.h

@@ -98,7 +98,17 @@ public:
 
     [[nodiscard]] auto compare(tr_interned_string const& that) const // <=>
     {
-        return this->quark() - that.quark();
+        if (this->quark() < that.quark())
+        {
+            return -1;
+        }
+
+        if (this->quark() > that.quark())
+        {
+            return 1;
+        }
+
+        return 0;
     }
 
     [[nodiscard]] bool operator<(tr_interned_string const& that) const

libtransmission/peer-io.cc

@@ -610,7 +610,7 @@ static tr_peerIo* tr_peerIoNew(
     tr_address const* addr,
     tr_port port,
     tr_sha1_digest_t const* torrent_hash,
-    bool isIncoming,
+    bool is_incoming,
     bool isSeed,
     struct tr_peer_socket const socket)
 {
@@ -630,8 +630,7 @@ static tr_peerIo* tr_peerIoNew(
         maybeSetCongestionAlgorithm(socket.handle.tcp, session->peerCongestionAlgorithm());
     }
 
-    auto* io = new tr_peerIo{ session, *addr, port, isSeed };
-    tr_cryptoConstruct(&io->crypto, torrent_hash, isIncoming);
+    auto* io = new tr_peerIo{ session, torrent_hash, is_incoming, *addr, port, isSeed };
     io->socket = socket;
     io->bandwidth = new Bandwidth(parent);
     io->bandwidth->setPeer(io);
@@ -653,7 +652,7 @@ static tr_peerIo* tr_peerIoNew(
         dbgmsg(io, "%s", "calling UTP_SetCallbacks &utp_function_table");
         UTP_SetCallbacks(socket.handle.utp, &utp_function_table, io);
 
-        if (!isIncoming)
+        if (!is_incoming)
         {
             dbgmsg(io, "%s", "calling UTP_Connect");
             UTP_Connect(socket.handle.utp);
@@ -875,7 +874,6 @@ static void io_dtor(void* vio)
     event_disable(io, EV_READ | EV_WRITE);
     delete io->bandwidth;
     io_close_socket(io);
-    tr_cryptoDestruct(&io->crypto);
 
     while (io->outbuf_datatypes != nullptr)
     {

libtransmission/peer-io.h

@@ -67,8 +67,15 @@ auto inline constexpr PEER_IO_MAGIC_NUMBER = 206745;
 class tr_peerIo
 {
 public:
-    tr_peerIo(tr_session* session_in, tr_address const& addr_in, tr_port port_in, bool is_seed_in)
-        : addr{ addr_in }
+    tr_peerIo(
+        tr_session* session_in,
+        tr_sha1_digest_t const* torrent_hash,
+        bool is_incoming,
+        tr_address const& addr_in,
+        tr_port port_in,
+        bool is_seed_in)
+        : crypto{ torrent_hash, is_incoming }
+        , addr{ addr_in }
         , session{ session_in }
         , inbuf{ evbuffer_new() }
         , outbuf{ evbuffer_new() }
@@ -83,7 +90,7 @@ public:
         evbuffer_free(inbuf);
     }
 
-    tr_crypto crypto = {};
+    tr_crypto crypto;
 
     tr_address const addr;
 
@@ -230,7 +237,7 @@ int tr_peerIoReconnect(tr_peerIo* io);
 
 constexpr bool tr_peerIoIsIncoming(tr_peerIo const* io)
 {
-    return io->crypto.isIncoming;
+    return io->crypto.is_incoming;
 }
 
 // TODO: remove this func; let caller get the current time instead

tests/libtransmission/crypto-test.cc

@@ -40,27 +40,22 @@ TEST(Crypto, torrentHash)
 {
 
     auto a = tr_crypto{};
-    tr_cryptoConstruct(&a, nullptr, true);
     EXPECT_FALSE(tr_cryptoGetTorrentHash(&a));
 
     tr_cryptoSetTorrentHash(&a, SomeHash);
     EXPECT_TRUE(tr_cryptoGetTorrentHash(&a));
     EXPECT_EQ(SomeHash, *tr_cryptoGetTorrentHash(&a));
-    tr_cryptoDestruct(&a);
 
-    tr_cryptoConstruct(&a, &SomeHash, false);
+    a = tr_crypto{ &SomeHash, false };
     EXPECT_TRUE(tr_cryptoGetTorrentHash(&a));
     EXPECT_EQ(SomeHash, *tr_cryptoGetTorrentHash(&a));
-
-    tr_cryptoDestruct(&a);
 }
 
 TEST(Crypto, encryptDecrypt)
 {
-    auto a = tr_crypto{};
-    tr_cryptoConstruct(&a, &SomeHash, false);
-    auto b = tr_crypto_{};
-    tr_cryptoConstruct_(&b, &SomeHash, true);
+    auto a = tr_crypto{ &SomeHash, false };
+    auto b = tr_crypto_{ &SomeHash, true };
+
     auto public_key_length = int{};
     EXPECT_TRUE(tr_cryptoComputeSecret(&a, tr_cryptoGetMyPublicKey_(&b, &public_key_length)));
     EXPECT_TRUE(tr_cryptoComputeSecret_(&b, tr_cryptoGetMyPublicKey(&a, &public_key_length)));
@@ -84,9 +79,6 @@ TEST(Crypto, encryptDecrypt)
     tr_cryptoDecryptInit(&a);
     tr_cryptoDecrypt(&a, input2.size(), encrypted2.data(), decrypted2.data());
     EXPECT_EQ(input2, std::string(decrypted2.data(), input2.size()));
-
-    tr_cryptoDestruct_(&b);
-    tr_cryptoDestruct(&a);
 }
 
 TEST(Crypto, sha1)