From 68bec3f8e3be2c4a5ff11303be1bd5c842f1918b Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Mon, 4 Apr 2022 13:36:48 -0500 Subject: [PATCH] refactor: remove tr_snprintf() (#2872) --- libtransmission/blocklist.cc | 94 ++++++++++++++--------------- libtransmission/clients.cc | 31 ++++++---- libtransmission/file-win32.cc | 4 +- libtransmission/tr-dht.cc | 2 +- libtransmission/tr-lpd.cc | 64 ++++++++------------ libtransmission/utils.cc | 88 ++++++++++----------------- libtransmission/utils.h | 3 - libtransmission/web-utils.cc | 4 +- tests/libtransmission/utils-test.cc | 37 +++--------- 9 files changed, 135 insertions(+), 192 deletions(-) diff --git a/libtransmission/blocklist.cc b/libtransmission/blocklist.cc index e062a9629..d1cf6fb40 100644 --- a/libtransmission/blocklist.cc +++ b/libtransmission/blocklist.cc @@ -7,6 +7,7 @@ #include #include /* bsearch(), qsort() */ #include +#include #include @@ -206,91 +207,84 @@ bool tr_blocklistFileHasAddress(tr_blocklistFile* b, tr_address const* addr) * http://wiki.phoenixlabs.org/wiki/P2P_Format * http://en.wikipedia.org/wiki/PeerGuardian#P2P_plaintext_format */ -static bool parseLine1(char const* line, struct tr_ipv4_range* range) +static bool parseLine1(std::string_view line, struct tr_ipv4_range* range) { - int b[4]; - int e[4]; - char str[64]; - tr_address addr; - - char const* walk = strrchr(line, ':'); - - if (walk == nullptr) + // remove leading "comment:" + auto pos = line.find(':'); + if (pos == std::string_view::npos) { return false; } + line = line.substr(pos + 1); - ++walk; /* walk past the colon */ - - if (sscanf( - walk, - "%d.%d.%d.%d-%d.%d.%d.%d", - TR_ARG_TUPLE(&b[0], &b[1], &b[2], &b[3]), - TR_ARG_TUPLE(&e[0], &e[1], &e[2], &e[3])) != 8) + // parse the leading 'x.x.x.x' + pos = line.find('-'); + if (pos == std::string_view::npos) { return false; } - - tr_snprintf(str, sizeof(str), "%d.%d.%d.%d", TR_ARG_TUPLE(b[0], b[1], b[2], b[3])); - - if (!tr_address_from_string(&addr, str)) + if (auto addr = tr_address{}; tr_address_from_string(&addr, line.substr(0, pos))) + { + range->begin = ntohl(addr.addr.addr4.s_addr); + } + else { return false; } + line = line.substr(pos + 1); - range->begin = ntohl(addr.addr.addr4.s_addr); - - tr_snprintf(str, sizeof(str), "%d.%d.%d.%d", TR_ARG_TUPLE(e[0], e[1], e[2], e[3])); - - if (!tr_address_from_string(&addr, str)) + // parse the trailing 'y.y.y.y' + if (auto addr = tr_address{}; tr_address_from_string(&addr, line)) + { + range->end = ntohl(addr.addr.addr4.s_addr); + } + else { return false; } - range->end = ntohl(addr.addr.addr4.s_addr); - return true; } /* - * DAT format: "000.000.000.000 - 000.255.255.255 , 000 , invalid ip" - * http://wiki.phoenixlabs.org/wiki/DAT_Format + * DAT / eMule format: "000.000.000.000 - 000.255.255.255 , 000 , invalid ip"a + * https://sourceforge.net/p/peerguardian/wiki/dev-blocklist-format-dat/ */ -static bool parseLine2(char const* line, struct tr_ipv4_range* range) +static bool parseLine2(std::string_view line, struct tr_ipv4_range* range) { - int unk = 0; - int a[4]; - int b[4]; - char str[32]; - tr_address addr; + static auto constexpr Delim1 = std::string_view{ " - " }; + static auto constexpr Delim2 = std::string_view{ " , " }; - if (sscanf( - line, - "%3d.%3d.%3d.%3d - %3d.%3d.%3d.%3d , %3d , ", - TR_ARG_TUPLE(&a[0], &a[1], &a[2], &a[3]), - TR_ARG_TUPLE(&b[0], &b[1], &b[2], &b[3]), - &unk) != 9) + auto pos = line.find(Delim1); + if (pos == std::string_view::npos) { return false; } - tr_snprintf(str, sizeof(str), "%d.%d.%d.%d", TR_ARG_TUPLE(a[0], a[1], a[2], a[3])); - - if (!tr_address_from_string(&addr, str)) + if (auto addr = tr_address{}; tr_address_from_string(&addr, line.substr(0, pos))) + { + range->begin = ntohl(addr.addr.addr4.s_addr); + } + else { return false; } - range->begin = ntohl(addr.addr.addr4.s_addr); - - tr_snprintf(str, sizeof(str), "%d.%d.%d.%d", TR_ARG_TUPLE(b[0], b[1], b[2], b[3])); - - if (!tr_address_from_string(&addr, str)) + line = line.substr(pos + std::size(Delim1)); + pos = line.find(Delim2); + if (pos == std::string_view::npos) { return false; } - range->end = ntohl(addr.addr.addr4.s_addr); + if (auto addr = tr_address{}; tr_address_from_string(&addr, line.substr(0, pos))) + { + range->end = ntohl(addr.addr.addr4.s_addr); + } + else + { + return false; + } return true; } diff --git a/libtransmission/clients.cc b/libtransmission/clients.cc index e7f768a24..3c256a120 100644 --- a/libtransmission/clients.cc +++ b/libtransmission/clients.cc @@ -16,9 +16,11 @@ #include #include +#include + #include "transmission.h" #include "clients.h" -#include "utils.h" /* tr_snprintf(), tr_strlcpy() */ +#include "utils.h" using namespace std::literals; // "foo"sv @@ -121,7 +123,7 @@ constexpr std::string_view getMnemonicEnd(uint8_t ch) void two_major_two_minor_formatter(char* buf, size_t buflen, std::string_view name, tr_peer_id_t id) { std::tie(buf, buflen) = buf_append(buf, buflen, name, ' ', strint(&id[3], 2), '.'); - tr_snprintf(buf, buflen, "%02d", strint(&id[5], 2)); + *fmt::format_to_n(buf, buflen - 1, FMT_STRING("{:02d}"), strint(&id[5], 2)).out = '\0'; } bool decodeShad0wClient(char* buf, size_t buflen, std::string_view in) @@ -239,7 +241,7 @@ bool decodeBitCometClient(char* buf, size_t buflen, std::string_view peer_id) int const minor = uint8_t(peer_id[5]); std::tie(buf, buflen) = buf_append(buf, buflen, name, ' ', mod, major, '.'); - tr_snprintf(buf, buflen, "%02d", minor); + *fmt::format_to_n(buf, buflen - 1, FMT_STRING("{:02d}"), minor).out = '\0'; return true; } @@ -301,7 +303,8 @@ constexpr void bitrocket_formatter(char* buf, size_t buflen, std::string_view na void bittorrent_dna_formatter(char* buf, size_t buflen, std::string_view name, tr_peer_id_t id) { std::tie(buf, buflen) = buf_append(buf, buflen, name, ' '); - tr_snprintf(buf, buflen, "%d.%d.%d", strint(&id[3], 2), strint(&id[5], 2), strint(&id[7], 2)); + *fmt::format_to_n(buf, buflen - 1, FMT_STRING("{:d}.{:d}.{:d}"), strint(&id[3], 2), strint(&id[5], 2), strint(&id[7], 2)) + .out = '\0'; } void bits_on_wheels_formatter(char* buf, size_t buflen, std::string_view name, tr_peer_id_t id) @@ -427,15 +430,22 @@ void transmission_formatter(char* buf, size_t buflen, std::string_view name, tr_ if (strncmp(&id[3], "000", 3) == 0) // very old client style: -TR0006- is 0.6 { - tr_snprintf(buf, buflen, "0.%c", id[6]); + *fmt::format_to_n(buf, buflen - 1, FMT_STRING("0.{:c}"), id[6]).out = '\0'; } else if (strncmp(&id[3], "00", 2) == 0) // previous client style: -TR0072- is 0.72 { - tr_snprintf(buf, buflen, "0.%02d", strint(&id[5], 2)); + *fmt::format_to_n(buf, buflen - 1, FMT_STRING("0.{:02d}"), strint(&id[5], 2)).out = '\0'; } else // current client style: -TR111Z- is 1.11+ */ { - tr_snprintf(buf, buflen, "%d.%02d%s", strint(&id[3], 1), strint(&id[4], 2), (id[6] == 'Z' || id[6] == 'X') ? "+" : ""); + *fmt::format_to_n( + buf, + buflen - 1, + FMT_STRING("{:d}.{:02d}{:s}"), + strint(&id[3], 1), + strint(&id[4], 2), + (id[6] == 'Z' || id[6] == 'X') ? "+" : "") + .out = '\0'; } } @@ -491,7 +501,7 @@ constexpr void xfplay_formatter(char* buf, size_t buflen, std::string_view name, void xtorrent_formatter(char* buf, size_t buflen, std::string_view name, tr_peer_id_t id) { std::tie(buf, buflen) = buf_append(buf, buflen, name, ' ', charints[id[3]], '.', charints[id[4]], " ("sv); - tr_snprintf(buf, buflen, "%d)", strint(&id[5], 2)); + *fmt::format_to_n(buf, buflen - 1, FMT_STRING("{:d}"), strint(&id[5], 2)).out = '\0'; } struct Client @@ -646,7 +656,7 @@ char* tr_clientForId(char* buf, size_t buflen, tr_peer_id_t peer_id) if (peer_id[0] == '\0' && peer_id[2] == 'B' && peer_id[3] == 'S') { - tr_snprintf(buf, buflen, "BitSpirit %d", peer_id[1] == '\0' ? 1 : int(peer_id[1])); + *fmt::format_to_n(buf, buflen - 1, FMT_STRING("BitSpirit {:d}"), peer_id[1] == '\0' ? 1 : int(peer_id[1])).out = '\0'; return buf; } @@ -689,8 +699,7 @@ char* tr_clientForId(char* buf, size_t buflen, tr_peer_id_t peer_id) } else { - tr_snprintf(walk, end - walk, "%%%02X", (unsigned int)c); - walk += 3; + walk = fmt::format_to_n(walk, end - walk - 1, FMT_STRING("%{:02X}"), unsigned(c)).out; } } diff --git a/libtransmission/file-win32.cc b/libtransmission/file-win32.cc index 76dbccf2c..fcea860d6 100644 --- a/libtransmission/file-win32.cc +++ b/libtransmission/file-win32.cc @@ -60,9 +60,7 @@ static void set_system_error(tr_error** error, DWORD code) } else { - auto buf = std::array{}; - tr_snprintf(std::data(buf), std::size(buf), "Unknown error: 0x%08lx", code); - tr_error_set(error, code, std::data(buf)); + tr_error_set(error, code, fmt::format(FMT_STRING("Unknown error: {:#08x}"), code)); } } diff --git a/libtransmission/tr-dht.cc b/libtransmission/tr-dht.cc index 906d7d76d..4c4c06663 100644 --- a/libtransmission/tr-dht.cc +++ b/libtransmission/tr-dht.cc @@ -819,7 +819,7 @@ int dht_sendto(int sockfd, void const* buf, int len, int flags, struct sockaddr #if defined(_WIN32) && !defined(__MINGW32__) -extern "C" int dht_gettimeofday(struct timeval* tv, struct timezone* tz) +extern "C" int dht_gettimeofday(struct timeval* tv, [[maybe_unused]] timezone* tz) { TR_ASSERT(tz == nullptr); *tv = tr_gettimeofday(); diff --git a/libtransmission/tr-lpd.cc b/libtransmission/tr-lpd.cc index 9ae3009be..1fd467936 100644 --- a/libtransmission/tr-lpd.cc +++ b/libtransmission/tr-lpd.cc @@ -37,6 +37,8 @@ using in_port_t = uint16_t; /* all missing */ #include "tr-lpd.h" #include "utils.h" +using namespace std::literals; + static auto constexpr SIZEOF_HASH_STRING = TR_SHA1_DIGEST_STRLEN; /** @@ -201,27 +203,16 @@ static bool lpd_extractParam(char const* const str, char const* const name, int TR_ASSERT(name != nullptr); TR_ASSERT(val != nullptr); - /* configure maximum length of search string here */ - auto constexpr MaxLength = int{ 30 }; + auto const key = tr_strbuf{ CRLF, name, ": "sv }; - char sstr[MaxLength] = { 0 }; - - if (strlen(name) > MaxLength - strlen(CRLF ": ")) - { - return false; - } - - /* compose the string token to search for */ - tr_snprintf(sstr, MaxLength, CRLF "%s: ", name); - - char const* const pos = strstr(str, sstr); + char const* const pos = strstr(str, key); if (pos == nullptr) { return false; /* search was not successful */ } { - char const* const beg = pos + strlen(sstr); + char const* const beg = pos + std::size(key); char const* const new_line = strstr(beg, CRLF); /* the value is delimited by the next CRLF */ @@ -437,38 +428,33 @@ void tr_lpdUninit(tr_session* ss) */ bool tr_lpdSendAnnounce(tr_torrent const* t) { - char constexpr fmt[] = // - "BT-SEARCH * HTTP/%u.%u" CRLF // - "Host: %s:%u" CRLF // - "Port: %u" CRLF // - "Infohash: %s" CRLF // - "" CRLF // - "" CRLF; - if (t == nullptr) { return false; } - /* ensure the hash string is capitalized */ - auto const hash_string = tr_strupper(t->infoHashString()); + auto const query = fmt::format( + FMT_STRING("BT-SEARCH * HTTP/{:d}.{:d}" CRLF "Host: {:s}:{:d}" CRLF "Port: {:d}" CRLF "Infohash: {:s}" CRLF CRLF CRLF), + 1, + 1, + lpd_mcastGroup, + lpd_mcastPort, + lpd_port, + tr_strupper(t->infoHashString())); - /* prepare a zero-terminated announce message */ - char query[lpd_maxDatagramLength + 1] = { 0 }; - tr_snprintf(query, lpd_maxDatagramLength + 1, fmt, 1, 1, lpd_mcastGroup, lpd_mcastPort, lpd_port, hash_string.c_str()); - - /* actually send the query out using [lpd_socket2] */ + // send the query out using [lpd_socket2] + // destination address info has already been set up in tr_lpdInit(), + // so we refrain from preparing another sockaddr_in here + if (auto const res = sendto( + lpd_socket2, + std::data(query), + std::size(query), + 0, + (struct sockaddr const*)&lpd_mcastAddr, + sizeof(lpd_mcastAddr)); + res != static_cast(std::size(query))) { - int const len = strlen(query); - - /* destination address info has already been set up in tr_lpdInit(), - * so we refrain from preparing another sockaddr_in here */ - int const res = sendto(lpd_socket2, query, len, 0, (struct sockaddr const*)&lpd_mcastAddr, sizeof(lpd_mcastAddr)); - - if (res != len) - { - return false; - } + return false; } tr_logAddTraceTor(t, "LPD announce message away"); diff --git a/libtransmission/utils.cc b/libtransmission/utils.cc index 519e8e6d7..b6692f35f 100644 --- a/libtransmission/utils.cc +++ b/libtransmission/utils.cc @@ -549,16 +549,6 @@ void tr_wait_msec(long int msec) **** ***/ -int tr_snprintf(void* buf, size_t buflen, char const* fmt, ...) -{ - va_list args; - - va_start(args, fmt); - int len = evutil_vsnprintf(static_cast(buf), buflen, fmt, args); - va_end(args); - return len; -} - /* * Copy src to string dst of size siz. At most siz-1 characters * will be copied. Always NUL terminates (unless siz == 0). @@ -1023,42 +1013,31 @@ std::vector tr_parseNumberRange(std::string_view str) double tr_truncd(double x, int precision) { - char buf[128]; - tr_snprintf(buf, sizeof(buf), "%.*f", TR_ARG_TUPLE(DBL_DIG, x)); + auto buf = std::array{}; + auto const [out, len] = fmt::format_to_n(std::data(buf), std::size(buf) - 1, "{:.{}f}", x, DBL_DIG); + *out = '\0'; - if (auto* const pt = strstr(buf, localeconv()->decimal_point); pt != nullptr) + if (auto* const pt = strstr(std::data(buf), localeconv()->decimal_point); pt != nullptr) { pt[precision != 0 ? precision + 1 : 0] = '\0'; } - return atof(buf); -} - -/* return a truncated double as a string */ -static char* tr_strtruncd(char* buf, double x, int precision, size_t buflen) -{ - tr_snprintf(buf, buflen, "%.*f", precision, tr_truncd(x, precision)); - return buf; + return atof(std::data(buf)); } std::string tr_strpercent(double x) { - auto buf = std::array{}; - if (x < 5.0) { - tr_strtruncd(std::data(buf), x, 2, std::size(buf)); - } - else if (x < 100.0) - { - tr_strtruncd(std::data(buf), x, 1, std::size(buf)); - } - else - { - tr_strtruncd(std::data(buf), x, 0, std::size(buf)); + return fmt::format("{:.2f}", tr_truncd(x, 2)); } - return std::data(buf); + if (x < 100.0) + { + return fmt::format("{:.1f}", tr_truncd(x, 1)); + } + + return fmt::format("{:.0f}", x); } std::string tr_strratio(double ratio, char const* infinity) @@ -1266,7 +1245,8 @@ static char* formatter_get_size_str(formatter_units const& u, char* buf, uint64_ precision = 1; } - tr_snprintf(buf, buflen, "%.*f %s", TR_ARG_TUPLE(precision, value), units); + auto const [out, len] = fmt::format_to_n(buf, buflen - 1, "{:.{}f} {:s}", value, precision, units); + *out = '\0'; return buf; } @@ -1295,33 +1275,27 @@ void tr_formatter_speed_init(size_t kilo, char const* kb, char const* mb, char c std::string tr_formatter_speed_KBps(double KBps) { - auto buf = std::array{}; + auto speed = KBps; - if (auto speed = KBps; speed <= 999.95) /* 0.0 KB to 999.9 KB */ + if (speed <= 999.95) // 0.0 KB to 999.9 KB { - tr_snprintf(std::data(buf), std::size(buf), "%d %s", int(speed), std::data(speed_units[TR_FMT_KB].name)); - } - else - { - double const K = speed_units[TR_FMT_KB].value; - - speed /= K; - - if (speed <= 99.995) /* 0.98 MB to 99.99 MB */ - { - tr_snprintf(std::data(buf), std::size(buf), "%.2f %s", speed, std::data(speed_units[TR_FMT_MB].name)); - } - else if (speed <= 999.95) /* 100.0 MB to 999.9 MB */ - { - tr_snprintf(std::data(buf), std::size(buf), "%.1f %s", speed, std::data(speed_units[TR_FMT_MB].name)); - } - else - { - tr_snprintf(std::data(buf), std::size(buf), "%.1f %s", speed / K, std::data(speed_units[TR_FMT_GB].name)); - } + return fmt::format("{:d} {:s}", int(speed), std::data(speed_units[TR_FMT_KB].name)); } - return std::data(buf); + double const K = speed_units[TR_FMT_KB].value; + speed /= K; + + if (speed <= 99.995) // 0.98 MB to 99.99 MB + { + return fmt::format("{:.2f} {:s}", speed, std::data(speed_units[TR_FMT_MB].name)); + } + + if (speed <= 999.95) // 100.0 MB to 999.9 MB + { + return fmt::format("{:.1f} {:s}", speed, std::data(speed_units[TR_FMT_MB].name)); + } + + return fmt::format("{:.1f} {:s}", speed / K, std::data(speed_units[TR_FMT_GB].name)); } static formatter_units mem_units; diff --git a/libtransmission/utils.h b/libtransmission/utils.h index c19fafe5c..579658ec8 100644 --- a/libtransmission/utils.h +++ b/libtransmission/utils.h @@ -264,9 +264,6 @@ std::string evbuffer_free_to_str(evbuffer* buf); /** @brief Portability wrapper for strlcpy() that uses the system implementation if available */ size_t tr_strlcpy(void* dst, void const* src, size_t siz); -/** @brief Portability wrapper for snprintf() that uses the system implementation if available */ -int tr_snprintf(void* buf, size_t buflen, char const* fmt, ...) TR_GNUC_PRINTF(3, 4) TR_GNUC_NONNULL(1, 3); - /** @brief Convenience wrapper around strerorr() guaranteed to not return nullptr @param errnum the error number to describe */ char const* tr_strerror(int errnum); diff --git a/libtransmission/web-utils.cc b/libtransmission/web-utils.cc index 7a02cdd87..2cd842edd 100644 --- a/libtransmission/web-utils.cc +++ b/libtransmission/web-utils.cc @@ -13,6 +13,8 @@ #include #include +#include + #define PSL_STATIC #include @@ -185,7 +187,7 @@ void tr_http_escape_sha1(char* out, tr_sha1_digest_t const& digest) } else { - out += tr_snprintf(out, 4, "%%%02x", (unsigned int)b); + out = fmt::format_to(out, FMT_STRING("%{:02x}"), unsigned(b)); } } diff --git a/tests/libtransmission/utils-test.cc b/tests/libtransmission/utils-test.cc index 46cb3a1e5..7c46883cb 100644 --- a/tests/libtransmission/utils-test.cc +++ b/tests/libtransmission/utils-test.cc @@ -297,37 +297,20 @@ TEST_F(UtilsTest, array) TEST_F(UtilsTest, truncd) { - auto buf = std::array{}; - - tr_snprintf(buf.data(), buf.size(), "%.2f%%", 99.999); - EXPECT_STREQ("100.00%", buf.data()); - - tr_snprintf(buf.data(), buf.size(), "%.2f%%", tr_truncd(99.999, 2)); - EXPECT_STREQ("99.99%", buf.data()); - - tr_snprintf(buf.data(), buf.size(), "%.4f", tr_truncd(403650.656250, 4)); - EXPECT_STREQ("403650.6562", buf.data()); - - tr_snprintf(buf.data(), buf.size(), "%.2f", tr_truncd(2.15, 2)); - EXPECT_STREQ("2.15", buf.data()); - - tr_snprintf(buf.data(), buf.size(), "%.2f", tr_truncd(2.05, 2)); - EXPECT_STREQ("2.05", buf.data()); - - tr_snprintf(buf.data(), buf.size(), "%.2f", tr_truncd(3.3333, 2)); - EXPECT_STREQ("3.33", buf.data()); - - tr_snprintf(buf.data(), buf.size(), "%.0f", tr_truncd(3.3333, 0)); - EXPECT_STREQ("3", buf.data()); - - tr_snprintf(buf.data(), buf.size(), "%.0f", tr_truncd(3.9999, 0)); - EXPECT_STREQ("3", buf.data()); + EXPECT_EQ("100.00%"sv, fmt::format("{:.2f}%", 99.999)); + EXPECT_EQ("99.99%"sv, fmt::format("{:.2f}%", tr_truncd(99.999, 2))); + EXPECT_EQ("403650.6562"sv, fmt::format("{:.4f}", tr_truncd(403650.656250, 4))); + EXPECT_EQ("2.15"sv, fmt::format("{:.2f}", tr_truncd(2.15, 2))); + EXPECT_EQ("2.05"sv, fmt::format("{:.2f}", tr_truncd(2.05, 2))); + EXPECT_EQ("3.33"sv, fmt::format("{:.2f}", tr_truncd(3.333333, 2))); + EXPECT_EQ("3"sv, fmt::format("{:.0f}", tr_truncd(3.333333, 0))); + EXPECT_EQ("3"sv, fmt::format("{:.0f}", tr_truncd(3.9999, 0))); #if !(defined(_MSC_VER) || (defined(__MINGW32__) && defined(__MSVCRT__))) /* FIXME: MSCVRT behaves differently in case of nan */ auto const nan = sqrt(-1.0); - tr_snprintf(buf.data(), buf.size(), "%.2f", tr_truncd(nan, 2)); - EXPECT_TRUE(strstr(buf.data(), "nan") != nullptr || strstr(buf.data(), "NaN") != nullptr); + auto const nanstr = fmt::format("{:.2f}", tr_truncd(nan, 2)); + EXPECT_TRUE(strstr(nanstr.c_str(), "nan") != nullptr || strstr(nanstr.c_str(), "NaN") != nullptr); #endif }