From d164343fe7bca45c9e7b6c4f5727a6ef7852bfca Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Sat, 9 Jul 2022 20:03:40 -0500 Subject: [PATCH] refactor: remove tr_address_to_string() (#3427) --- libtransmission/net.cc | 65 +++++++----------------------- libtransmission/net.h | 23 +++++------ libtransmission/peer-mgr.cc | 4 +- libtransmission/port-forwarding.cc | 7 +--- libtransmission/session.cc | 8 ++-- libtransmission/upnp.cc | 12 +++--- libtransmission/upnp.h | 4 +- 7 files changed, 40 insertions(+), 83 deletions(-) diff --git a/libtransmission/net.cc b/libtransmission/net.cc index 409af90af..d338d980a 100644 --- a/libtransmission/net.cc +++ b/libtransmission/net.cc @@ -61,34 +61,6 @@ std::string tr_net_strerror(int err) #endif } -char const* tr_address_and_port_to_string(char* buf, size_t buflen, tr_address const* addr, tr_port port) -{ - char addr_buf[INET6_ADDRSTRLEN]; - tr_address_to_string_with_buf(addr, addr_buf, sizeof(addr_buf)); - *fmt::format_to_n(buf, buflen - 1, FMT_STRING("[{:s}]:{:d}"), addr_buf, port.host()).out = '\0'; - return buf; -} - -char const* tr_address_to_string_with_buf(tr_address const* addr, char* buf, size_t buflen) -{ - TR_ASSERT(tr_address_is_valid(addr)); - - return addr->type == TR_AF_INET ? evutil_inet_ntop(AF_INET, &addr->addr, buf, buflen) : - evutil_inet_ntop(AF_INET6, &addr->addr, buf, buflen); -} - -/* - * Non-threadsafe version of tr_address_to_string_with_buf() - * and uses a static memory area for a buffer. - * This function is suitable to be called from libTransmission's networking code, - * which is single-threaded. - */ -char const* tr_address_to_string(tr_address const* addr) -{ - static char buf[INET6_ADDRSTRLEN]; - return tr_address_to_string_with_buf(addr, buf, sizeof(buf)); -} - bool tr_address_from_string(tr_address* dst, char const* src) { if (evutil_inet_pton(AF_INET, src, &dst->addr) == 1) @@ -416,9 +388,7 @@ struct tr_peer_socket tr_netOpenPeerSocket(tr_session* session, tr_address const ret = tr_peer_socket_tcp_create(s); } - char addrstr[TR_ADDRSTRLEN]; - tr_address_and_port_to_string(addrstr, sizeof(addrstr), addr, port); - tr_logAddTrace(fmt::format("New OUTGOING connection {} ({})", s, addrstr)); + tr_logAddTrace(fmt::format("New OUTGOING connection {} ({})", s, addr->readable(port))); return ret; } @@ -903,38 +873,33 @@ std::optional tr_address::fromString(std::string_view address_str) return addr; } -template -OutputIt tr_address::readable(OutputIt out) const +std::string_view tr_address::readable(char* out, size_t outlen, tr_port port) const { + if (std::empty(port)) + { + return isIPv4() ? evutil_inet_ntop(AF_INET, &addr, out, outlen) : evutil_inet_ntop(AF_INET6, &addr, out, outlen); + } + auto buf = std::array{}; - tr_address_to_string_with_buf(this, std::data(buf), std::size(buf)); - return fmt::format_to(out, FMT_STRING("{:s}"), std::data(buf)); -} - -template char* tr_address::readable(char*) const; - -std::string tr_address::readable() const -{ - auto buf = std::string{}; - buf.reserve(INET6_ADDRSTRLEN); - this->readable(std::back_inserter(buf)); - return buf; + auto const addr_sv = readable(std::data(buf), std::size(buf)); + auto const [end, size] = fmt::format_to_n(out, outlen - 1, FMT_STRING("[{:s}]:{:d}"), addr_sv, port.host()); + return { out, size }; } template OutputIt tr_address::readable(OutputIt out, tr_port port) const { - auto buf = std::array{}; - tr_address_to_string_with_buf(this, std::data(buf), std::size(buf)); - return fmt::format_to(out, FMT_STRING("[{:s}]:{:d}"), std::data(buf), port.host()); + auto addrbuf = std::array{}; + auto const addr_sv = readable(std::data(addrbuf), std::size(addrbuf), port); + return std::copy(std::begin(addr_sv), std::end(addr_sv), out); } template char* tr_address::readable(char*, tr_port) const; -std::string tr_address::readable(tr_port port) const +[[nodiscard]] std::string tr_address::readable(tr_port port) const { auto buf = std::string{}; - buf.reserve(INET6_ADDRSTRLEN + 9); + buf.reserve(INET6_ADDRSTRLEN + 16); this->readable(std::back_inserter(buf), port); return buf; } diff --git a/libtransmission/net.h b/libtransmission/net.h index ac68b6b32..039a089e6 100644 --- a/libtransmission/net.h +++ b/libtransmission/net.h @@ -156,15 +156,20 @@ struct tr_address [[nodiscard]] static std::pair fromCompact6(uint8_t const* compact) noexcept; // human-readable formatting - template - OutputIt readable(OutputIt out) const; + OutputIt readable(OutputIt out, tr_port port = {}) const; + std::string_view readable(char* out, size_t outlen, tr_port port = {}) const; + [[nodiscard]] std::string readable(tr_port port = {}) const; - template - OutputIt readable(OutputIt out, tr_port) const; + [[nodiscard]] constexpr auto isIPv4() const noexcept + { + return type == TR_AF_INET; + } - [[nodiscard]] std::string readable() const; - [[nodiscard]] std::string readable(tr_port) const; + [[nodiscard]] constexpr auto isIPv6() const noexcept + { + return type == TR_AF_INET6; + } // comparisons @@ -196,12 +201,6 @@ struct tr_address extern tr_address const tr_inaddr_any; extern tr_address const tr_in6addr_any; -char const* tr_address_to_string(tr_address const* addr); - -char const* tr_address_to_string_with_buf(tr_address const* addr, char* buf, size_t buflen); - -char const* tr_address_and_port_to_string(char* buf, size_t buflen, tr_address const* addr, tr_port port); - bool tr_address_from_string(tr_address* setme, char const* string); bool tr_address_from_string(tr_address* dst, std::string_view src); diff --git a/libtransmission/peer-mgr.cc b/libtransmission/peer-mgr.cc index 80fe9d41e..ddf0152e6 100644 --- a/libtransmission/peer-mgr.cc +++ b/libtransmission/peer-mgr.cc @@ -1184,7 +1184,7 @@ void tr_peerMgrAddIncoming(tr_peerMgr* manager, tr_address const* addr, tr_port if (tr_sessionIsAddressBlocked(session, addr)) { - tr_logAddTrace(fmt::format("Banned IP address '{}' tried to connect to us", tr_address_to_string(addr))); + tr_logAddTrace(fmt::format("Banned IP address '{}' tried to connect to us", addr->readable(port))); tr_netClosePeerSocket(session, socket); } else if (manager->incoming_handshakes.contains(*addr)) @@ -1631,7 +1631,7 @@ namespace peer_stat_helpers auto const [addr, port] = peer->socketAddress(); - tr_address_to_string_with_buf(&addr, stats.addr, sizeof(stats.addr)); + addr.readable(stats.addr, sizeof(stats.addr)); stats.client = peer->client.c_str(); stats.port = port.host(); stats.from = atom->fromFirst; diff --git a/libtransmission/port-forwarding.cc b/libtransmission/port-forwarding.cc index 8e4f36adf..20813589b 100644 --- a/libtransmission/port-forwarding.cc +++ b/libtransmission/port-forwarding.cc @@ -96,12 +96,7 @@ static void natPulse(tr_shared* s, bool do_check) fmt::arg("private_port", session->private_peer_port.host()))); } - s->upnpStatus = tr_upnpPulse( - s->upnp, - private_peer_port, - is_enabled, - do_check, - tr_address_to_string(&session->bind_ipv4->addr)); + s->upnpStatus = tr_upnpPulse(s->upnp, private_peer_port, is_enabled, do_check, session->bind_ipv4->addr.readable()); auto const new_status = tr_sharedTraversalStatus(s); diff --git a/libtransmission/session.cc b/libtransmission/session.cc index 7b65835a5..42343f18d 100644 --- a/libtransmission/session.cc +++ b/libtransmission/session.cc @@ -246,9 +246,7 @@ static void accept_incoming_peer(evutil_socket_t fd, short /*what*/, void* vsess if (clientSocket != TR_BAD_SOCKET) { - char addrstr[TR_ADDRSTRLEN]; - tr_address_and_port_to_string(addrstr, sizeof(addrstr), &clientAddr, clientPort); - tr_logAddTrace(fmt::format("new incoming connection {} ({})", clientSocket, addrstr)); + tr_logAddTrace(fmt::format("new incoming connection {} ({})", clientSocket, clientAddr.readable(clientPort))); tr_peerMgrAddIncoming(session->peerMgr, &clientAddr, clientPort, tr_peer_socket_tcp_create(clientSocket)); } @@ -464,8 +462,8 @@ void tr_sessionGetSettings(tr_session const* s, tr_variant* d) tr_variantDictAddBool(d, TR_KEY_speed_limit_up_enabled, tr_sessionIsSpeedLimited(s, TR_UP)); tr_variantDictAddStr(d, TR_KEY_umask, fmt::format("{:#o}", s->umask)); tr_variantDictAddInt(d, TR_KEY_upload_slots_per_torrent, s->uploadSlotsPerTorrent); - tr_variantDictAddStr(d, TR_KEY_bind_address_ipv4, tr_address_to_string(&s->bind_ipv4->addr)); - tr_variantDictAddStr(d, TR_KEY_bind_address_ipv6, tr_address_to_string(&s->bind_ipv6->addr)); + tr_variantDictAddStr(d, TR_KEY_bind_address_ipv4, s->bind_ipv4->addr.readable()); + tr_variantDictAddStr(d, TR_KEY_bind_address_ipv6, s->bind_ipv6->addr.readable()); tr_variantDictAddBool(d, TR_KEY_start_added_torrents, !tr_sessionGetPaused(s)); tr_variantDictAddBool(d, TR_KEY_trash_original_torrent_files, tr_sessionGetDeleteSource(s)); tr_variantDictAddInt(d, TR_KEY_anti_brute_force_threshold, tr_sessionGetAntiBruteForceThreshold(s)); diff --git a/libtransmission/upnp.cc b/libtransmission/upnp.cc index 6a3ad4587..64e784b8a 100644 --- a/libtransmission/upnp.cc +++ b/libtransmission/upnp.cc @@ -240,11 +240,9 @@ enum UPNP_IGD_INVALID = 3 }; -static auto* discoverThreadfunc(char* bindaddr) +static auto* discoverThreadfunc(std::string bindaddr) { - auto* const ret = tr_upnpDiscover(2000, bindaddr); - tr_free(bindaddr); - return ret; + return tr_upnpDiscover(2000, bindaddr.c_str()); } template @@ -253,17 +251,17 @@ static bool isFutureReady(std::future const& future) return future.wait_for(std::chrono::seconds(0)) == std::future_status::ready; } -tr_port_forwarding tr_upnpPulse(tr_upnp* handle, tr_port port, bool isEnabled, bool doPortCheck, char const* bindaddr) +tr_port_forwarding tr_upnpPulse(tr_upnp* handle, tr_port port, bool isEnabled, bool doPortCheck, std::string bindaddr) { if (isEnabled && handle->state == UpnpState::WILL_DISCOVER) { TR_ASSERT(!handle->discover_future); - auto task = std::packaged_task{ discoverThreadfunc }; + auto task = std::packaged_task{ discoverThreadfunc }; handle->discover_future = task.get_future(); handle->state = UpnpState::DISCOVERING; - std::thread(std::move(task), tr_strdup(bindaddr)).detach(); + std::thread(std::move(task), std::move(bindaddr)).detach(); } if (isEnabled && handle->state == UpnpState::DISCOVERING && handle->discover_future && diff --git a/libtransmission/upnp.h b/libtransmission/upnp.h index d575f5a55..545c8e031 100644 --- a/libtransmission/upnp.h +++ b/libtransmission/upnp.h @@ -14,6 +14,8 @@ * @{ */ +#include + #include "transmission.h" #include "net.h" // tr_port @@ -24,6 +26,6 @@ tr_upnp* tr_upnpInit(void); void tr_upnpClose(tr_upnp*); -tr_port_forwarding tr_upnpPulse(tr_upnp*, tr_port port, bool isEnabled, bool doPortCheck, char const*); +tr_port_forwarding tr_upnpPulse(tr_upnp*, tr_port port, bool isEnabled, bool doPortCheck, std::string); /* @} */