build: add clang tidy modernize loop convert (#1949)

* build: add clang-tidy check modernize-pass-by-value

* build: add clang-tidy check modernize-loop-convert

* refactor: constexpr

* refactor: make preallocGuess a std::array

libtransmission/.clang-tidy

@@ -2,8 +2,10 @@
 # Many of these checks are disabled only because the code hasn't been
 # cleaned up yet. Pull requests welcomed.
 Checks: >
+  modernize-loop-convert,
   modernize-pass-by-value,
   modernize-use-default-member-init,
   modernize-use-nullptr,
-  modernize-use-override
+  modernize-use-override,
+  modernize-use-using
 

libtransmission/announcer.cc

@@ -7,12 +7,14 @@
  */
 
 #include <algorithm>
+#include <array>
 #include <climits> /* INT_MAX */
 #include <cstdio>
 #include <cstdlib> /* qsort() */
 #include <cstring> /* strcmp(), memcpy(), strncmp() */
 #include <map>
 #include <set>
+#include <string_view>
 #include <vector>
 
 #include <event2/buffer.h>
@@ -1298,24 +1300,19 @@ static void tierAnnounce(tr_announcer* announcer, tr_tier* tier)
 ****
 ***/
 
-static bool multiscrape_too_big(char const* errmsg)
+static constexpr bool multiscrape_too_big(std::string_view errmsg)
 {
     /* Found a tracker that returns some bespoke string for this case?
        Add your patch here and open a PR */
-    static char const* const too_long_errors[] = {
+    auto constexpr TooLongErrors = std::array<std::string_view, 3>{
         "Bad Request",
         "GET string too long",
         "Request-URI Too Long",
     };
 
-    if (errmsg == nullptr)
+    for (auto const& tle : TooLongErrors)
     {
-        return false;
+        if (errmsg.find(tle) != std::string_view::npos)
-    }
-
-    for (size_t i = 0; i < TR_N_ELEMENTS(too_long_errors); ++i)
-    {
-        if (strstr(errmsg, too_long_errors[i]) != nullptr)
         {
             return true;
         }
@@ -1459,7 +1456,7 @@ static void on_scrape_done(tr_scrape_response const* response, void* vsession)
     }
 
     /* Maybe reduce the number of torrents in a multiscrape req */
-    if (multiscrape_too_big(response->errmsg.c_str()))
+    if (multiscrape_too_big(response->errmsg))
     {
         auto const& url = response->url;
         struct tr_scrape_info* const scrape_info = tr_announcerGetScrapeInfo(announcer, url);

libtransmission/crypto-utils.cc

@@ -146,9 +146,9 @@ char* tr_ssha1(char const* plain_text)
 
     tr_rand_buffer(salt, saltval_len);
 
-    for (size_t i = 0; i < saltval_len; ++i)
+    for (auto& ch : salt)
     {
-        salt[i] = salter[salt[i] % salter_len];
+        ch = salter[ch % salter_len];
     }
 
     tr_sha1(sha, plain_text, (int)strlen(plain_text), salt, saltval_len, nullptr);

libtransmission/metainfo.cc

@@ -9,6 +9,7 @@
 #include <algorithm>
 #include <array>
 #include <string.h> /* strlen() */
+#include <string_view>
 
 #include <event2/buffer.h>
 
@@ -107,10 +108,9 @@ char* tr_metainfo_sanitize_path_component(char const* str, size_t len, bool* is_
 
     /* https://docs.microsoft.com/en-us/windows/desktop/FileIO/naming-a-file */
     char const* const reserved_chars = "<>:\"/\\|?*";
-    char const* const reserved_names[] = {
+    auto constexpr ReservedNames = std::array<std::string_view, 22>{
-        "CON",  "PRN",  "AUX",  "NUL", //
+        "CON",  "PRN",  "AUX",  "NUL",  "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7",
-        "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", "COM8", "COM9", //
+        "COM8", "COM9", "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9",
-        "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9", //
     };
 
     char* const ret = tr_new(char, len + 2);
@@ -126,10 +126,10 @@ char* tr_metainfo_sanitize_path_component(char const* str, size_t len, bool* is_
         }
     }
 
-    for (size_t i = 0; i < TR_N_ELEMENTS(reserved_names); ++i)
+    for (auto const& reserved_name : ReservedNames)
     {
-        size_t const reserved_name_len = strlen(reserved_names[i]);
+        size_t const reserved_name_len = std::size(reserved_name);
-        if (evutil_ascii_strncasecmp(ret, reserved_names[i], reserved_name_len) != 0 ||
+        if (evutil_ascii_strncasecmp(ret, std::data(reserved_name), reserved_name_len) != 0 ||
             (ret[reserved_name_len] != '\0' && ret[reserved_name_len] != '.'))
         {
             continue;

libtransmission/torrent.cc

@@ -2899,30 +2899,29 @@ uint64_t tr_torrentGetBytesLeftToAllocate(tr_torrent const* tor)
 *****  Removing the torrent's local data
 ****/
 
-static bool isJunkFile(char const* base)
+static constexpr bool isJunkFile(std::string_view base)
 {
-    static char const* files[] = {
+    auto constexpr Files = std::array<std::string_view, 3>{
         ".DS_Store",
-        "desktop.ini",
         "Thumbs.db",
+        "desktop.ini",
     };
 
-    for (size_t i = 0; i < TR_N_ELEMENTS(files); ++i)
+    // TODO(C++20): std::any_of is constexpr in C++20
+    for (auto const& file : Files)
     {
-        if (strcmp(base, files[i]) == 0)
+        if (file == base)
         {
             return true;
         }
     }
 
 #ifdef __APPLE__
-
+    // check for resource forks. <http://support.apple.com/kb/TA20578>
-    /* check for resource forks. <http://support.apple.com/kb/TA20578> */
+    if (base.find("._") == 0)
-    if (memcmp(base, "._", 2) == 0)
     {
         return true;
     }
-
 #endif
 
     return false;

libtransmission/utils.cc

@@ -1620,10 +1620,13 @@ struct formatter_unit
     size_t value;
 };
 
+using formatter_units = std::array<formatter_unit, 4>;
+/*
 struct formatter_units
 {
     struct formatter_unit units[4];
 };
+*/
 
 enum
 {
@@ -1633,60 +1636,52 @@ enum
     TR_FMT_TB
 };
 
-static void formatter_init(
+static void formatter_init(formatter_units& units, size_t kilo, char const* kb, char const* mb, char const* gb, char const* tb)
-    struct formatter_units* units,
-    size_t kilo,
-    char const* kb,
-    char const* mb,
-    char const* gb,
-    char const* tb)
 {
     size_t value;
 
     value = kilo;
-    units->units[TR_FMT_KB].name = tr_strdup(kb);
+    units[TR_FMT_KB].name = tr_strdup(kb);
-    units->units[TR_FMT_KB].value = value;
+    units[TR_FMT_KB].value = value;
 
     value *= kilo;
-    units->units[TR_FMT_MB].name = tr_strdup(mb);
+    units[TR_FMT_MB].name = tr_strdup(mb);
-    units->units[TR_FMT_MB].value = value;
+    units[TR_FMT_MB].value = value;
 
     value *= kilo;
-    units->units[TR_FMT_GB].name = tr_strdup(gb);
+    units[TR_FMT_GB].name = tr_strdup(gb);
-    units->units[TR_FMT_GB].value = value;
+    units[TR_FMT_GB].value = value;
 
     value *= kilo;
-    units->units[TR_FMT_TB].name = tr_strdup(tb);
+    units[TR_FMT_TB].name = tr_strdup(tb);
-    units->units[TR_FMT_TB].value = value;
+    units[TR_FMT_TB].value = value;
 }
 
-static char* formatter_get_size_str(struct formatter_units const* u, char* buf, size_t bytes, size_t buflen)
+static char* formatter_get_size_str(formatter_units const& u, char* buf, size_t bytes, size_t buflen)
 {
-    int precision;
+    formatter_unit const* unit;
-    double value;
-    char const* units;
-    struct formatter_unit const* unit;
 
-    if (bytes < u->units[1].value)
+    if (bytes < u[1].value)
     {
-        unit = &u->units[0];
+        unit = &u[0];
     }
-    else if (bytes < u->units[2].value)
+    else if (bytes < u[2].value)
     {
-        unit = &u->units[1];
+        unit = &u[1];
     }
-    else if (bytes < u->units[3].value)
+    else if (bytes < u[3].value)
     {
-        unit = &u->units[2];
+        unit = &u[2];
     }
     else
     {
-        unit = &u->units[3];
+        unit = &u[3];
     }
 
-    value = (double)bytes / unit->value;
+    double value = (double)bytes / unit->value;
-    units = unit->name;
+    char const* units = unit->name;
 
+    int precision = 0;
     if (unit->value == 1)
     {
         precision = 0;
@@ -1704,36 +1699,36 @@ static char* formatter_get_size_str(struct formatter_units const* u, char* buf,
     return buf;
 }
 
-static struct formatter_units size_units;
+static formatter_units size_units;
 
 void tr_formatter_size_init(size_t kilo, char const* kb, char const* mb, char const* gb, char const* tb)
 {
-    formatter_init(&size_units, kilo, kb, mb, gb, tb);
+    formatter_init(size_units, kilo, kb, mb, gb, tb);
 }
 
 char* tr_formatter_size_B(char* buf, size_t bytes, size_t buflen)
 {
-    return formatter_get_size_str(&size_units, buf, bytes, buflen);
+    return formatter_get_size_str(size_units, buf, bytes, buflen);
 }
 
-static struct formatter_units speed_units;
+static formatter_units speed_units;
 
 size_t tr_speed_K = 0;
 
 void tr_formatter_speed_init(size_t kilo, char const* kb, char const* mb, char const* gb, char const* tb)
 {
     tr_speed_K = kilo;
-    formatter_init(&speed_units, kilo, kb, mb, gb, tb);
+    formatter_init(speed_units, kilo, kb, mb, gb, tb);
 }
 
 char* tr_formatter_speed_KBps(char* buf, double KBps, size_t buflen)
 {
-    double const K = speed_units.units[TR_FMT_KB].value;
+    double const K = speed_units[TR_FMT_KB].value;
     double speed = KBps;
 
     if (speed <= 999.95) /* 0.0 KB to 999.9 KB */
     {
-        tr_snprintf(buf, buflen, "%d %s", (int)speed, speed_units.units[TR_FMT_KB].name);
+        tr_snprintf(buf, buflen, "%d %s", (int)speed, speed_units[TR_FMT_KB].name);
     }
     else
     {
@@ -1741,34 +1736,34 @@ char* tr_formatter_speed_KBps(char* buf, double KBps, size_t buflen)
 
         if (speed <= 99.995) /* 0.98 MB to 99.99 MB */
         {
-            tr_snprintf(buf, buflen, "%.2f %s", speed, speed_units.units[TR_FMT_MB].name);
+            tr_snprintf(buf, buflen, "%.2f %s", speed, speed_units[TR_FMT_MB].name);
         }
         else if (speed <= 999.95) /* 100.0 MB to 999.9 MB */
         {
-            tr_snprintf(buf, buflen, "%.1f %s", speed, speed_units.units[TR_FMT_MB].name);
+            tr_snprintf(buf, buflen, "%.1f %s", speed, speed_units[TR_FMT_MB].name);
         }
         else
         {
-            tr_snprintf(buf, buflen, "%.1f %s", speed / K, speed_units.units[TR_FMT_GB].name);
+            tr_snprintf(buf, buflen, "%.1f %s", speed / K, speed_units[TR_FMT_GB].name);
         }
     }
 
     return buf;
 }
 
-static struct formatter_units mem_units;
+static formatter_units mem_units;
 
 size_t tr_mem_K = 0;
 
 void tr_formatter_mem_init(size_t kilo, char const* kb, char const* mb, char const* gb, char const* tb)
 {
     tr_mem_K = kilo;
-    formatter_init(&mem_units, kilo, kb, mb, gb, tb);
+    formatter_init(mem_units, kilo, kb, mb, gb, tb);
 }
 
 char* tr_formatter_mem_B(char* buf, size_t bytes_per_second, size_t buflen)
 {
-    return formatter_get_size_str(&mem_units, buf, bytes_per_second, buflen);
+    return formatter_get_size_str(mem_units, buf, bytes_per_second, buflen);
 }
 
 void tr_formatter_get_units(void* vdict)
@@ -1778,28 +1773,25 @@ void tr_formatter_get_units(void* vdict)
 
     tr_variantDictReserve(dict, 6);
 
-    tr_variantDictAddInt(dict, TR_KEY_memory_bytes, mem_units.units[TR_FMT_KB].value);
+    tr_variantDictAddInt(dict, TR_KEY_memory_bytes, mem_units[TR_FMT_KB].value);
-    l = tr_variantDictAddList(dict, TR_KEY_memory_units, 4);
+    l = tr_variantDictAddList(dict, TR_KEY_memory_units, std::size(mem_units));
-
+    for (auto const& unit : mem_units)
-    for (int i = 0; i < 4; i++)
     {
-        tr_variantListAddStr(l, mem_units.units[i].name);
+        tr_variantListAddStr(l, unit.name);
     }
 
-    tr_variantDictAddInt(dict, TR_KEY_size_bytes, size_units.units[TR_FMT_KB].value);
+    tr_variantDictAddInt(dict, TR_KEY_size_bytes, size_units[TR_FMT_KB].value);
-    l = tr_variantDictAddList(dict, TR_KEY_size_units, 4);
+    l = tr_variantDictAddList(dict, TR_KEY_size_units, std::size(size_units));
-
+    for (auto const& unit : size_units)
-    for (int i = 0; i < 4; i++)
     {
-        tr_variantListAddStr(l, size_units.units[i].name);
+        tr_variantListAddStr(l, unit.name);
     }
 
-    tr_variantDictAddInt(dict, TR_KEY_speed_bytes, speed_units.units[TR_FMT_KB].value);
+    tr_variantDictAddInt(dict, TR_KEY_speed_bytes, speed_units[TR_FMT_KB].value);
-    l = tr_variantDictAddList(dict, TR_KEY_speed_units, 4);
+    l = tr_variantDictAddList(dict, TR_KEY_speed_units, std::size(speed_units));
-
+    for (auto const& unit : speed_units)
-    for (int i = 0; i < 4; i++)
     {
-        tr_variantListAddStr(l, speed_units.units[i].name);
+        tr_variantListAddStr(l, unit.name);
     }
 }
 

libtransmission/variant-json.cc

@@ -6,9 +6,10 @@
  *
  */
 
+#include <array>
 #include <ctype.h>
-#include <errno.h> /* EILSEQ, EINVAL */
 #include <deque>
+#include <errno.h> /* EILSEQ, EINVAL */
 #include <math.h> /* fabs() */
 #include <stdio.h>
 #include <string.h>
@@ -46,7 +47,7 @@ struct json_wrapper_data
      * e.g. they may all be objects with the same set of keys. So when
      * a container is popped off the stack, remember its size to use as
      * a preallocation heuristic for the next container at that depth. */
-    size_t preallocGuess[MAX_DEPTH];
+    std::array<size_t, MAX_DEPTH> preallocGuess;
 };
 
 static tr_variant* get_node(struct jsonsl_st* jsn)
@@ -384,10 +385,7 @@ int tr_jsonParse(char const* source, void const* vbuf, size_t len, tr_variant* s
     data.source = source;
     data.keybuf = evbuffer_new();
     data.strbuf = evbuffer_new();
-    for (int i = 0; i < MAX_DEPTH; ++i)
+    data.preallocGuess = {};
-    {
-        data.preallocGuess[i] = 0;
-    }
 
     /* parse it */
     jsonsl_feed(jsn, static_cast<jsonsl_char_t const*>(vbuf), len);