From 9950adf472349e849966e8d467118b92ed90aceb Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Wed, 11 Aug 2021 00:12:26 -0300 Subject: [PATCH] Remove unneeded split(str, char) method This function had a bug in stable (fixed in dev) when `last` returns npos, but the function also appears to basically be duplicating what the next split version can do, so this just removes it and uses the single more generic split(strview, strview) method. --- llarp/config/config.cpp | 2 +- llarp/net/route.cpp | 2 +- llarp/net/sock_addr.cpp | 4 +-- llarp/util/str.cpp | 29 ---------------------- llarp/util/str.hpp | 8 ------ test/net/test_sock_addr.cpp | 4 +-- test/util/test_llarp_util_str.cpp | 41 +++++++++++++++++++++++-------- 7 files changed, 37 insertions(+), 53 deletions(-) diff --git a/llarp/config/config.cpp b/llarp/config/config.cpp index bef3e521f..78d152602 100644 --- a/llarp/config/config.cpp +++ b/llarp/config/config.cpp @@ -798,7 +798,7 @@ namespace llarp { info.interface = std::string{name}; - std::vector splits = split(value, ','); + std::vector splits = split(value, ","); for (std::string_view str : splits) { int asNum = std::atoi(str.data()); diff --git a/llarp/net/route.cpp b/llarp/net/route.cpp index 109823fb3..604208465 100644 --- a/llarp/net/route.cpp +++ b/llarp/net/route.cpp @@ -558,7 +558,7 @@ namespace llarp::net std::ifstream inf("/proc/net/route"); for (std::string line; std::getline(inf, line);) { - const auto parts = split(line, '\t'); + const auto parts = split(line, "\t"); if (parts[1].find_first_not_of('0') == std::string::npos and parts[0] != ifname) { const auto& ip = parts[2]; diff --git a/llarp/net/sock_addr.cpp b/llarp/net/sock_addr.cpp index 2fd6ddf5a..ee6817414 100644 --- a/llarp/net/sock_addr.cpp +++ b/llarp/net/sock_addr.cpp @@ -244,7 +244,7 @@ namespace llarp // NOTE: this potentially involves multiple memory allocations, // reimplement without split() if it is performance bottleneck - auto splits = split(str, ':'); + auto splits = split(str, ":"); // TODO: having ":port" at the end makes this ambiguous with IPv6 // come up with a strategy for implementing @@ -260,7 +260,7 @@ namespace llarp assert(splits.size() > 0); // splits[0] should be dot-separated IPv4 - auto ipSplits = split(splits[0], '.'); + auto ipSplits = split(splits[0], "."); if (ipSplits.size() != 4) throw std::invalid_argument(stringify(str, " is not a valid IPv4 address")); diff --git a/llarp/util/str.cpp b/llarp/util/str.cpp index 60090c3ad..512d5c55b 100644 --- a/llarp/util/str.cpp +++ b/llarp/util/str.cpp @@ -67,35 +67,6 @@ namespace llarp return str; } - std::vector - split(const std::string_view str, char delimiter) - { - std::vector splits; - const auto str_size = str.size(); - size_t last = 0; - size_t next = 0; - while (last < str_size and next < std::string_view::npos) - { - next = str.find_first_of(delimiter, last); - if (next > last) - { - splits.push_back(str.substr(last, next - last)); - - last = next; - - // advance to next non-delimiter - while (last < str_size and str[last] == delimiter) - last++; - } - else - { - break; - } - } - - return splits; - } - using namespace std::literals; std::vector diff --git a/llarp/util/str.hpp b/llarp/util/str.hpp index ab6e4588d..2fdebe838 100644 --- a/llarp/util/str.hpp +++ b/llarp/util/str.hpp @@ -35,14 +35,6 @@ namespace llarp return o.str(); } - /// Split a string on a given delimiter - // - /// @param str is the string to split - /// @param delimiter is the character to split on - /// @return a vector of std::string_views with the split words, excluding the delimeter - std::vector - split(const std::string_view str, char delimiter); - using namespace std::literals; /// Returns true if the first string is equal to the second string, compared case-insensitively. diff --git a/test/net/test_sock_addr.cpp b/test/net/test_sock_addr.cpp index 9102a96ed..c0b666745 100644 --- a/test/net/test_sock_addr.cpp +++ b/test/net/test_sock_addr.cpp @@ -43,9 +43,9 @@ TEST_CASE("SockAddr fromString", "[SockAddr]") CHECK_THROWS_WITH(llarp::SockAddr("1.2.3"), "1.2.3 is not a valid IPv4 address"); - CHECK_THROWS_WITH(llarp::SockAddr("1.2.3."), "1.2.3. is not a valid IPv4 address"); + CHECK_THROWS_WITH(llarp::SockAddr("1.2.3."), "1.2.3. contains invalid numeric value"); - CHECK_THROWS_WITH(llarp::SockAddr(".1.2.3"), ".1.2.3 is not a valid IPv4 address"); + CHECK_THROWS_WITH(llarp::SockAddr(".1.2.3"), ".1.2.3 contains invalid numeric value"); CHECK_THROWS_WITH(llarp::SockAddr("1.2.3.4.5"), "1.2.3.4.5 is not a valid IPv4 address"); diff --git a/test/util/test_llarp_util_str.cpp b/test/util/test_llarp_util_str.cpp index b7cd15fba..b53173660 100644 --- a/test/util/test_llarp_util_str.cpp +++ b/test/util/test_llarp_util_str.cpp @@ -91,7 +91,7 @@ TEST_CASE("neither true nor false string values", "[str][nottruefalse]") { } TEST_CASE("split strings with multiple matches", "[str]") { - auto splits = llarp::split("this is a test", ' '); + auto splits = llarp::split("this is a test", " "); REQUIRE(splits.size() == 4); REQUIRE(splits[0] == "this"); REQUIRE(splits[1] == "is"); @@ -100,13 +100,13 @@ TEST_CASE("split strings with multiple matches", "[str]") { } TEST_CASE("split strings with single match", "[str]") { - auto splits = llarp::split("uno", ';'); + auto splits = llarp::split("uno", ";"); REQUIRE(splits.size() == 1); REQUIRE(splits[0] == "uno"); } -TEST_CASE("split strings with consecutive delimiters", "[str]") { - auto splits = llarp::split("a o e u", ' '); +TEST_CASE("split_any strings with consecutive delimiters", "[str]") { + auto splits = llarp::split_any("a o e u", " "); REQUIRE(splits.size() == 4); REQUIRE(splits[0] == "a"); REQUIRE(splits[1] == "o"); @@ -115,14 +115,35 @@ TEST_CASE("split strings with consecutive delimiters", "[str]") { } TEST_CASE("split delimiter-only string", "[str]") { - auto splits = llarp::split(" ", ' '); - REQUIRE(splits.size() == 0); + { + auto splits = llarp::split(" ", " "); + REQUIRE(splits.size() == 5); + } - splits = llarp::split(" ", ' '); - REQUIRE(splits.size() == 0); + { + auto splits = llarp::split_any(" ", " "); + REQUIRE(splits.size() == 2); + } + + { + auto splits = llarp::split(" ", " ", true); + REQUIRE(splits.size() == 0); + } + + { + auto splits = llarp::split_any(" ", " ", true); + REQUIRE(splits.size() == 0); + } } TEST_CASE("split empty string", "[str]") { - auto splits = llarp::split("", ' '); - REQUIRE(splits.size() == 0); + { + auto splits = llarp::split("", " "); + REQUIRE(splits.size() == 1); + } + + { + auto splits = llarp::split("", " ", true); + REQUIRE(splits.size() == 0); + } }