refactor: tr_parseNumberRange returns std::vector. (#1838)

* refactor: tr_parseNumberRange returns std::vector.

libtransmission/rpcimpl.cc

@@ -1382,7 +1382,7 @@ static char const* removeTrackers(tr_torrent* tor, tr_variant* ids)
     }
 
     /* sort trackerIds and remove from largest to smallest so there is no need to recalculate array indicies */
-    qsort(tids, t, sizeof(int), compareInt);
+    std::sort(tids, tids + t);
 
     bool changed = false;
     int dup = -1;
@@ -2917,8 +2917,8 @@ void tr_rpc_request_exec_json(
  */
 void tr_rpc_parse_list_str(tr_variant* setme, char const* str, size_t len)
 {
-    int valueCount;
+    auto const values = tr_parseNumberRange(str, len);
-    int* values = tr_parseNumberRange(str, len, &valueCount);
+    auto const valueCount = std::size(values);
 
     if (valueCount == 0)
     {
@@ -2932,13 +2932,11 @@ void tr_rpc_parse_list_str(tr_variant* setme, char const* str, size_t len)
     {
         tr_variantInitList(setme, valueCount);
 
-        for (int i = 0; i < valueCount; ++i)
+        for (auto const& value : values)
         {
-            tr_variantListAddInt(setme, values[i]);
+            tr_variantListAddInt(setme, value);
         }
     }
-
-    tr_free(values);
 }
 
 void tr_rpc_request_exec_uri(

libtransmission/utils.cc

@@ -12,6 +12,7 @@
 #endif
 #endif
 
+#include <algorithm> // std::sort
 #include <array> // std::array
 #include <cctype> /* isdigit(), tolower() */
 #include <cerrno>
@@ -23,6 +24,17 @@
 #include <cstdlib> /* getenv() */
 #include <cstring> /* strerror(), memset(), memmem() */
 #include <ctime> /* nanosleep() */
+#include <exception>
+#include <set>
+#include <string>
+#include <vector>
+
+#if defined(__GNUC__) && !__has_include(<charconv>)
+#undef HAVE_CHARCONV
+#else
+#define HAVE_CHARCONV 1
+#include <charconv> // std::from_chars()
+#endif
 
 #ifdef _WIN32
 #include <ws2tcpip.h> /* WSAStartup() */
@@ -1393,61 +1405,46 @@ struct number_range
  * This should be a single number (ex. "6") or a range (ex. "6-9").
  * Anything else is an error and will return failure.
  */
-static bool parseNumberSection(char const* str, size_t len, struct number_range* setme)
+static bool parseNumberSection(char const* str, char const* const end, number_range& range)
 {
-    long a;
-    long b;
     bool success;
-    char* end;
+    auto const error = errno;
-    int const error = errno;
-    char* tmp = tr_strndup(str, len);
 
-    errno = 0;
+#if defined(HAVE_CHARCONV)
-    a = b = strtol(tmp, &end, 10);
+    auto result = std::from_chars(str, end, range.low);
-
+    success = result.ec == std::errc{};
-    if (errno != 0 || end == tmp)
+    if (success)
+    {
+        range.high = range.low;
+        if (result.ptr != end && *result.ptr == '-')
+        {
+            result = std::from_chars(result.ptr + 1, end, range.high);
+            success = result.ec == std::errc{};
+        }
+    }
+#else
+    try
+    {
+        auto tmp = std::string(str, end);
+        auto pos = size_t{};
+        range.low = range.high = std::stoi(tmp, &pos);
+        if (pos != std::size(tmp) && tmp[pos] == '-')
+        {
+            tmp.erase(0, pos + 1);
+            range.high = std::stoi(tmp, &pos);
+        }
+        success = true;
+    }
+    catch (std::exception&)
     {
         success = false;
     }
-    else if (*end != '-')
+#endif
-    {
-        success = true;
-    }
-    else
-    {
-        char const* pch = end + 1;
-        b = strtol(pch, &end, 10);
-
-        if (errno != 0 || pch == end)
-        {
-            success = false;
-        }
-        else if (*end != '\0') /* trailing data */
-        {
-            success = false;
-        }
-        else
-        {
-            success = true;
-        }
-    }
-
-    tr_free(tmp);
-
-    setme->low = (int)std::min(a, b); // FIXME: narrowing long to int
-    setme->high = (int)std::max(a, b);
 
     errno = error;
     return success;
 }
 
-int compareInt(void const* va, void const* vb)
-{
-    int const a = *(int const*)va;
-    int const b = *(int const*)vb;
-    return a - b;
-}
-
 /**
  * Given a string like "1-4" or "1-4,6,9,14-51", this allocates and returns an
  * array of setmeCount ints of all the values in the array.
@@ -1455,106 +1452,29 @@ int compareInt(void const* va, void const* vb)
  * It's the caller's responsibility to call tr_free () on the returned array.
  * If a fragment of the string can't be parsed, NULL is returned.
  */
-int* tr_parseNumberRange(char const* str_in, size_t len, int* setmeCount)
+std::vector<int> tr_parseNumberRange(char const* str, size_t len) // TODO: string_view
 {
-    int n = 0;
+    auto values = std::set<int>{};
-    int* uniq = nullptr;
-    char* str = tr_strndup(str_in, len);
-    char const* walk;
-    tr_list* ranges = nullptr;
-    bool success = true;
 
-    walk = str;
+    auto const* const end = str + (len != TR_BAD_SIZE ? len : strlen(str));
-
+    for (auto const* walk = str; walk < end;)
-    while (!tr_str_is_empty(walk) && success)
     {
-        struct number_range range;
+        auto delim = std::find(walk, end, ',');
-        char const* pch = strchr(walk, ',');
+        auto range = number_range{};
-
+        if (!parseNumberSection(walk, delim, range))
-        if (pch != nullptr)
         {
-            success = parseNumberSection(walk, (size_t)(pch - walk), &range);
+            break;
-            walk = pch + 1;
-        }
-        else
-        {
-            success = parseNumberSection(walk, strlen(walk), &range);
-            walk += strlen(walk);
         }
 
-        if (success)
+        for (auto i = range.low; i <= range.high; ++i)
         {
-            tr_list_append(&ranges, tr_memdup(&range, sizeof(struct number_range)));
+            values.insert(i);
         }
+
+        walk = delim + 1;
     }
 
-    if (!success)
+    return { std::begin(values), std::end(values) };
-    {
-        *setmeCount = 0;
-        uniq = nullptr;
-    }
-    else
-    {
-        int n2;
-        int* sorted = nullptr;
-
-        /* build a sorted number array */
-        n = n2 = 0;
-
-        for (tr_list* l = ranges; l != nullptr; l = l->next)
-        {
-            auto const* r = static_cast<struct number_range const*>(l->data);
-            n += r->high + 1 - r->low;
-        }
-
-        sorted = tr_new(int, n);
-
-        if (sorted == nullptr)
-        {
-            n = 0;
-            uniq = nullptr;
-        }
-        else
-        {
-            for (tr_list* l = ranges; l != nullptr; l = l->next)
-            {
-                auto const* r = static_cast<struct number_range const*>(l->data);
-
-                for (int i = r->low; i <= r->high; ++i)
-                {
-                    sorted[n2++] = i;
-                }
-            }
-
-            qsort(sorted, n, sizeof(int), compareInt);
-            TR_ASSERT(n == n2);
-
-            /* remove duplicates */
-            uniq = tr_new(int, n);
-            n = 0;
-
-            if (uniq != nullptr)
-            {
-                for (int i = 0; i < n2; ++i)
-                {
-                    if (n == 0 || uniq[n - 1] != sorted[i])
-                    {
-                        uniq[n++] = sorted[i];
-                    }
-                }
-            }
-
-            tr_free(sorted);
-        }
-    }
-
-    /* cleanup */
-    tr_list_free(&ranges, tr_free);
-    tr_free(str);
-
-    /* return the result */
-    *setmeCount = n;
-    return uniq;
 }
 
 /***

libtransmission/utils.h

@@ -12,6 +12,7 @@
 #include <stdarg.h>
 #include <stddef.h> /* size_t */
 #include <time.h> /* time_t */
+#include <vector>
 
 #include "tr-macros.h"
 
@@ -256,8 +257,6 @@ char* tr_strjoin(char const* const* arr, size_t len, char const* delim);
 ****
 ***/
 
-int compareInt(void const* va, void const* vb);
-
 void tr_binary_to_hex(void const* input, void* output, size_t byte_length) TR_GNUC_NONNULL(1, 2);
 void tr_hex_to_binary(void const* input, void* output, size_t byte_length) TR_GNUC_NONNULL(1, 2);
 
@@ -287,7 +286,7 @@ double tr_getRatio(uint64_t numerator, uint64_t denominator);
  *
  * For example, "5-8" will return [ 5, 6, 7, 8 ] and setmeCount will be 4.
  */
-int* tr_parseNumberRange(char const* str, size_t str_len, int* setmeCount) TR_GNUC_MALLOC TR_GNUC_NONNULL(1);
+std::vector<int> tr_parseNumberRange(char const* str, size_t str_len) TR_GNUC_NONNULL(1);
 
 /**
  * @brief truncate a double value at a given number of decimal places.

libtransmission/wildmat.c

@@ -37,8 +37,7 @@
 **  on.
 */
 
-#include "transmission.h"
+#include <stdbool.h>
-#include "utils.h"
 
 #define ABORT -1
 

tests/libtransmission/utils-test.cc

@@ -25,6 +25,8 @@
 #include <array>
 #include <cmath> // sqrt()
 #include <cstdlib> // setenv(), unsetenv()
+#include <iostream>
+#include <sstream>
 #include <string>
 
 using ::libtransmission::test::makeString;
@@ -136,38 +138,31 @@ TEST_F(UtilsTest, trUtf8clean)
 
 TEST_F(UtilsTest, numbers)
 {
-    auto count = int{};
+    auto const tostring = [](std::vector<int> const& v)
-    auto* numbers = tr_parseNumberRange("1-10,13,16-19", TR_BAD_SIZE, &count);
-    EXPECT_EQ(15, count);
-    EXPECT_EQ(1, numbers[0]);
-    EXPECT_EQ(6, numbers[5]);
-    EXPECT_EQ(10, numbers[9]);
-    EXPECT_EQ(13, numbers[10]);
-    EXPECT_EQ(16, numbers[11]);
-    EXPECT_EQ(19, numbers[14]);
-    tr_free(numbers);
-
-    numbers = tr_parseNumberRange("1-5,3-7,2-6", TR_BAD_SIZE, &count);
-    EXPECT_EQ(7, count);
-    EXPECT_NE(nullptr, numbers);
-    for (int i = 0; i < count; ++i)
     {
-        EXPECT_EQ(i + 1, numbers[i]);
+        std::stringstream ss;
-    }
+        for (auto const& i : v)
+        {
+            ss << i << ' ';
+        }
+        return ss.str();
+    };
 
-    tr_free(numbers);
+    auto numbers = tr_parseNumberRange("1-10,13,16-19", TR_BAD_SIZE);
+    EXPECT_EQ(std::string("1 2 3 4 5 6 7 8 9 10 13 16 17 18 19 "), tostring(numbers));
 
-    numbers = tr_parseNumberRange("1-Hello", TR_BAD_SIZE, &count);
+    numbers = tr_parseNumberRange("1-5,3-7,2-6", TR_BAD_SIZE);
-    EXPECT_EQ(0, count);
+    EXPECT_EQ(std::string("1 2 3 4 5 6 7 "), tostring(numbers));
-    EXPECT_EQ(nullptr, numbers);
 
-    numbers = tr_parseNumberRange("1-", TR_BAD_SIZE, &count);
+    numbers = tr_parseNumberRange("1-Hello", TR_BAD_SIZE);
-    EXPECT_EQ(0, count);
+    auto const empty_string = std::string{};
-    EXPECT_EQ(nullptr, numbers);
+    EXPECT_EQ(empty_string, tostring(numbers));
 
-    numbers = tr_parseNumberRange("Hello", TR_BAD_SIZE, &count);
+    numbers = tr_parseNumberRange("1-", TR_BAD_SIZE);
-    EXPECT_EQ(0, count);
+    EXPECT_EQ(empty_string, tostring(numbers));
-    EXPECT_EQ(nullptr, numbers);
+
+    numbers = tr_parseNumberRange("Hello", TR_BAD_SIZE);
+    EXPECT_EQ(empty_string, tostring(numbers));
 }
 
 namespace

utils/remote.cc

@@ -640,27 +640,20 @@ static void addDays(tr_variant* args, tr_quark const key, char const* arg)
 
     if (arg != NULL)
     {
-        int valueCount;
+        for (int& day : tr_parseNumberRange(arg, TR_BAD_SIZE))
-        int* values;
-
-        values = tr_parseNumberRange(arg, TR_BAD_SIZE, &valueCount);
-
-        for (int i = 0; i < valueCount; ++i)
         {
-            if (values[i] < 0 || values[i] > 7)
+            if (day < 0 || day > 7)
             {
                 continue;
             }
 
-            if (values[i] == 7)
+            if (day == 7)
             {
-                values[i] = 0;
+                day = 0;
             }
 
-            days |= 1 << values[i];
+            days |= 1 << day;
         }
-
-        tr_free(values);
     }
 
     if (days != 0)
@@ -708,15 +701,10 @@ static void addFiles(tr_variant* args, tr_quark const key, char const* arg)
 
     if (strcmp(arg, "all") != 0)
     {
-        int valueCount;
+        for (auto const& idx : tr_parseNumberRange(arg, TR_BAD_SIZE))
-        int* values = tr_parseNumberRange(arg, TR_BAD_SIZE, &valueCount);
-
-        for (int i = 0; i < valueCount; ++i)
         {
-            tr_variantListAddInt(files, values[i]);
+            tr_variantListAddInt(files, idx);
         }
-
-        tr_free(values);
     }
 }