refactor: don't store torrent hashes in QStrings (#1428)

* refactor: keep torrent hash in std::array<char,20>

This replaces the hashString QString allocation with a compile-time
array that's included in sizeof the Torrent struct that owns it.

qt/DetailsDialog.cc

@@ -794,20 +794,17 @@ void DetailsDialog::refreshUI()
     ui_.sizeValueLabel->setText(string);
 
     // myHashLabel
-    string = none;
-
-    if (!torrents.empty())
+    if (torrents.empty())
     {
-        string = torrents[0]->hashString();
-
-        for (Torrent const* const t : torrents)
-        {
-            if (string != t->hashString())
-            {
-                string = mixed;
-                break;
-            }
-        }
+        string = none;
+    }
+    else if (torrents.size() > 1)
+    {
+        string = mixed;
+    }
+    else
+    {
+        string = torrents.front()->hash().toString();
     }
 
     ui_.hashValueLabel->setText(string);

qt/Torrent.cc

@@ -222,7 +222,7 @@ Torrent::fields_t Torrent::update(tr_quark const* keys, tr_variant const* const*
             HANDLE_KEY(eta, eta, ETA)
             HANDLE_KEY(fileStats, files, FILES)
             HANDLE_KEY(files, files, FILES)
-            HANDLE_KEY(hashString, hash_string, HASH_STRING)
+            HANDLE_KEY(hashString, hash, HASH)
             HANDLE_KEY(haveUnchecked, have_unchecked, HAVE_UNCHECKED)
             HANDLE_KEY(haveValid, have_verified, HAVE_VERIFIED)
             HANDLE_KEY(honorsSessionLimits, honors_session_limits, HONORS_SESSION_LIMITS)

qt/Torrent.h

@@ -8,6 +8,7 @@
 
 #pragma once
 
+#include <array>
 #include <bitset>
 #include <ctime> // time_t
 
@@ -17,6 +18,7 @@
 #include <QString>
 
 #include <libtransmission/transmission.h>
+#include <libtransmission/crypto-utils.h>
 #include <libtransmission/quark.h>
 
 #include "FaviconCache.h"
@@ -84,7 +86,6 @@ struct TrackerStat
     int tier;
     FaviconCache::Key favicon_key;
     QString announce;
-    QString host;
     QString last_announce_result;
     QString last_scrape_result;
 };
@@ -103,6 +104,47 @@ struct TorrentFile
 
 using FileList = QVector<TorrentFile>;
 
+class TorrentHash
+{
+private:
+    std::array<uint8_t, SHA_DIGEST_LENGTH> data_ = {};
+
+public:
+    TorrentHash() {}
+
+    TorrentHash(char const* str)
+    {
+        tr_hex_to_sha1(data_.data(), str);
+    }
+
+    TorrentHash(QString const& str)
+    {
+        tr_hex_to_sha1(data_.data(), str.toUtf8().constData());
+    }
+
+    bool operator ==(TorrentHash const& that) const
+    {
+        return data_ == that.data_;
+    }
+
+    bool operator !=(TorrentHash const& that) const
+    {
+        return data_ != that.data_;
+    }
+
+    bool operator <(TorrentHash const& that) const
+    {
+        return data_ < that.data_;
+    }
+
+    QString toString() const
+    {
+        char str[SHA_DIGEST_LENGTH * 2 + 1];
+        tr_sha1_to_hex(str, data_.data());
+        return QString::fromUtf8(str, SHA_DIGEST_LENGTH * 2);
+    }
+};
+
 class Torrent : public QObject
 {
     Q_OBJECT
@@ -148,9 +190,9 @@ public:
 
     QString getError() const;
 
-    QString const& hashString() const
+    TorrentHash const& hash() const
     {
-        return hash_string_;
+        return hash_;
     }
 
     bool hasError() const
@@ -523,7 +565,7 @@ public:
         ETA,
         FAILED_EVER,
         FILES,
-        HASH_STRING,
+        HASH,
         HAVE_UNCHECKED,
         HAVE_VERIFIED,
         HONORS_SESSION_LIMITS,
@@ -622,7 +664,6 @@ private:
     QString creator_;
     QString download_dir_;
     QString error_string_;
-    QString hash_string_;
     QString name_;
 
     QIcon icon_;
@@ -637,6 +678,8 @@ private:
     Speed download_speed_;
 
     Prefs const& prefs_;
+
+    TorrentHash hash_;
 };
 
 Q_DECLARE_METATYPE(Torrent const*)

qt/TorrentFilter.cc

@@ -221,7 +221,7 @@ bool TorrentFilter::lessThan(QModelIndex const& left, QModelIndex const& right)
 
     if (val == 0)
     {
-        val = compare(a->hashString(), b->hashString());
+        val = compare(a->hash(), b->hash());
     }
 
     return val < 0;
@@ -253,9 +253,7 @@ bool TorrentFilter::filterAcceptsRow(int source_row, QModelIndex const& source_p
     if (accepts)
     {
         auto const text = prefs_.getString(Prefs::FILTER_TEXT);
-        accepts = text.isEmpty() ||
-            tor.name().contains(text, Qt::CaseInsensitive) ||
-            tor.hashString().contains(text, Qt::CaseInsensitive);
+        accepts = text.isEmpty() || tor.name().contains(text, Qt::CaseInsensitive);
     }
 
     return accepts;

qt/TorrentModel.cc

@@ -485,8 +485,8 @@ void TorrentModel::rowsRemove(torrents_t const& torrents)
 ****
 ***/
 
-bool TorrentModel::hasTorrent(QString const& hash_string) const
+bool TorrentModel::hasTorrent(TorrentHash const& hash) const
 {
-    auto test = [hash_string](auto const& tor) { return tor->hashString() == hash_string; };
+    auto test = [hash](auto const& tor) { return tor->hash() == hash; };
     return std::any_of(torrents_.cbegin(), torrents_.cend(), test);
 }

qt/TorrentModel.h

@@ -41,7 +41,7 @@ public:
     virtual ~TorrentModel() override;
     void clear();
 
-    bool hasTorrent(QString const& hash_string) const;
+    bool hasTorrent(TorrentHash const& hash) const;
 
     Torrent* getTorrentFromId(int id);
     Torrent const* getTorrentFromId(int id) const;

qt/VariantHelpers.cc

@@ -39,6 +39,12 @@ bool change(Speed& setme, tr_variant const* variant)
     return value && change(setme, Speed::fromBps(*value));
 }
 
+bool change(TorrentHash& setme, tr_variant const* value)
+{
+    auto const hash_string = getValue<std::string_view>(value);
+    return hash_string && change(setme, TorrentHash(hash_string->data()));
+}
+
 bool change(Peer& setme, tr_variant const* value)
 {
     bool changed = false;
@@ -130,7 +136,6 @@ bool change(TrackerStat& setme, tr_variant const* value)
             HANDLE_KEY(downloadCount, download_count)
             HANDLE_KEY(hasAnnounced, has_announced)
             HANDLE_KEY(hasScraped, has_scraped)
-            HANDLE_KEY(host, host)
             HANDLE_KEY(id, id)
             HANDLE_KEY(isBackup, is_backup)
             HANDLE_KEY(lastAnnouncePeerCount, last_announce_peer_count)

qt/VariantHelpers.h

@@ -19,6 +19,7 @@
 class QByteArray;
 
 class Speed;
+class TorrentHash;
 struct Peer;
 struct TorrentFile;
 struct TrackerStat;
@@ -88,6 +89,20 @@ auto getValue(tr_variant const* variant)
     return ret;
 }
 
+template<typename T, typename std::enable_if<std::is_same_v<T, std::string_view>>::type* = nullptr>
+auto getValue(tr_variant const* variant)
+{
+    std::optional<T> ret;
+    char const* str;
+    size_t len;
+    if (tr_variantGetStr(variant, &str, &len))
+    {
+        ret = std::string_view(str, len);
+    }
+
+    return ret;
+}
+
 template<typename T>
 bool change(T& setme, T const& value)
 {
@@ -105,6 +120,7 @@ bool change(double& setme, double const& value);
 bool change(Speed& setme, tr_variant const* value);
 bool change(Peer& setme, tr_variant const* value);
 bool change(TorrentFile& setme, tr_variant const* value);
+bool change(TorrentHash& setme, tr_variant const* value);
 bool change(TrackerStat& setme, tr_variant const* value);
 
 template<typename T>

qt/WatchDir.cc

@@ -45,7 +45,7 @@ int WatchDir::metainfoTest(QString const& filename) const
     {
         ret = ERROR;
     }
-    else if (model_.hasTorrent(QString::fromUtf8(inf.hashString)))
+    else if (model_.hasTorrent(TorrentHash(inf.hashString)))
     {
         ret = DUPLICATE;
     }

tests/libtransmission/clients-test.cc

@@ -15,14 +15,14 @@
 
 TEST(Client, clientForId)
 {
-    struct Test_local
+    struct LocalTest
     {
         char const* peer_id;
         char const* expected_client;
     };
 
-    auto constexpr Tests = std::array<Test_local, 24>{
-        Test_local{ "-BT791B-", "BitTorrent 7.9.1 (Beta)" },
+    auto constexpr Tests = std::array<LocalTest, 24>{
+        LocalTest{ "-BT791B-", "BitTorrent 7.9.1 (Beta)" },
         { "-BT791\0-", "BitTorrent 7.9.1" },
         { "-FC1013-", "FileCroc 1.0.1.3" },
         { "-FC1013-", "FileCroc 1.0.1.3" },

tests/libtransmission/crypto-test.cc

@@ -122,14 +122,14 @@ TEST(Crypto, sha1)
 
 TEST(Crypto, ssha1)
 {
-    struct Test_local
+    struct LocalTest
     {
         char const* const plain_text;
         char const* const ssha1;
     };
 
-    auto constexpr Tests = std::array<Test_local, 2>{
-        Test_local{ "test", "{15ad0621b259a84d24dcd4e75b09004e98a3627bAMbyRHJy" },
+    auto constexpr Tests = std::array<LocalTest, 2>{
+        LocalTest{ "test", "{15ad0621b259a84d24dcd4e75b09004e98a3627bAMbyRHJy" },
         { "QNY)(*#$B)!_X$B !_B#($^!)*&$%CV!#)&$C!@$(P*)", "{10e2d7acbb104d970514a147cd16d51dfa40fb3c0OSwJtOL" }
     };
 

tests/libtransmission/file-test.cc

@@ -976,15 +976,15 @@ TEST_F(FileTest, pathNativeSeparators)
 {
     EXPECT_EQ(nullptr, tr_sys_path_native_separators(nullptr));
 
-    struct Test_local
+    struct LocalTest
     {
         std::string input;
         std::string expected_output;
     };
 
-    auto const tests = std::array<Test_local, 5>
+    auto const tests = std::array<LocalTest, 5>
     {
-        Test_local{ "", "" },
+        LocalTest{ "", "" },
         { "a", TR_IF_WIN32("a", "a") },
         { "/", TR_IF_WIN32("\\", "/") },
         { "/a/b/c", TR_IF_WIN32("\\a\\b\\c", "/a/b/c") },

tests/libtransmission/metainfo-test.cc

@@ -52,15 +52,15 @@ TEST(Metainfo, magnetLink)
 // FIXME: split these into parameterized tests?
 TEST(Metainfo, bucket)
 {
-    struct Test_local
+    struct LocalTest
     {
         int expected_benc_err;
         int expected_parse_result;
         void const* benc;
     };
 
-    auto constexpr Tests = std::array<Test_local, 9>{
-        Test_local{ 0, TR_PARSE_OK, BEFORE_PATH "5:a.txt" AFTER_PATH },
+    auto constexpr Tests = std::array<LocalTest, 9>{
+        LocalTest{ 0, TR_PARSE_OK, BEFORE_PATH "5:a.txt" AFTER_PATH },
 
         /* allow empty components, but not =all= empty components, see bug #5517 */
         { 0, TR_PARSE_OK, BEFORE_PATH "0:5:a.txt" AFTER_PATH },
@@ -101,7 +101,7 @@ TEST(Metainfo, bucket)
 
 TEST(Metainfo, sanitize)
 {
-    struct Test_local
+    struct LocalTest
     {
         char const* str;
         size_t len;
@@ -109,9 +109,9 @@ TEST(Metainfo, sanitize)
         bool expected_is_adjusted;
     };
 
-    auto constexpr Tests = std::array<Test_local, 29>{
+    auto constexpr Tests = std::array<LocalTest, 29>{
         // skipped
-        Test_local{ "", 0, nullptr, false },
+        LocalTest{ "", 0, nullptr, false },
         { ".", 1, nullptr, false },
         { "..", 2, nullptr, true },
         { ".....", 5, nullptr, false },

tests/libtransmission/variant-test.cc

@@ -249,14 +249,14 @@ TEST_F(VariantTest, parse)
 }
 
 TEST_F(VariantTest, bencParseAndReencode) {
-    struct Test_local
+    struct LocalTest
     {
         std::string benc;
         bool is_good;
     };
 
-    auto const tests = std::array<Test_local, 9>{
-        Test_local{ "llleee", true },
+    auto const tests = std::array<LocalTest, 9>{
+        LocalTest{ "llleee", true },
         { "d3:cow3:moo4:spam4:eggse", true },
         { "d4:spaml1:a1:bee", true },
         { "d5:greenli1ei2ei3ee4:spamd1:ai123e3:keyi214eee", true },
@@ -343,14 +343,14 @@ TEST_F(VariantTest, bencMalformedIncompleteString)
 
 TEST_F(VariantTest, bencToJson)
 {
-    struct Test_local
+    struct LocalTest
     {
         std::string benc;
         std::string expected;
     };
 
-    auto const tests = std::array<Test_local, 5>{
-        Test_local{ "i6e", "6" },
+    auto const tests = std::array<LocalTest, 5>{
+        LocalTest{ "i6e", "6" },
         { "d5:helloi1e5:worldi2ee", R"({"hello":1,"world":2})" },
         { "d5:helloi1e5:worldi2e3:fooli1ei2ei3eee", R"({"foo":[1,2,3],"hello":1,"world":2})" },
         { "d5:helloi1e5:worldi2e3:fooli1ei2ei3ed1:ai0eeee", R"({"foo":[1,2,3,{"a":0}],"hello":1,"world":2})" },