refactor: remove tr_address_to_string() (#3427)

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_logAddTrace(fmt::format("New OUTGOING connection {} ({})", s, addr->readable(port)));
-    tr_address_and_port_to_string(addrstr, sizeof(addrstr), addr, port);
-    tr_logAddTrace(fmt::format("New OUTGOING connection {} ({})", s, addrstr));
 
     return ret;
 }
@@ -903,38 +873,33 @@ std::optional<tr_address> tr_address::fromString(std::string_view address_str)
     return addr;
 }
 
-template<typename OutputIt>
+std::string_view tr_address::readable(char* out, size_t outlen, tr_port port) const
-OutputIt tr_address::readable(OutputIt out) 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<char, INET6_ADDRSTRLEN>{};
-    tr_address_to_string_with_buf(this, std::data(buf), std::size(buf));
+    auto const addr_sv = readable(std::data(buf), std::size(buf));
-    return fmt::format_to(out, FMT_STRING("{:s}"), std::data(buf));
+    auto const [end, size] = fmt::format_to_n(out, outlen - 1, FMT_STRING("[{:s}]:{:d}"), addr_sv, port.host());
-}
+    return { out, size };
-
-template char* tr_address::readable<char*>(char*) const;
-
-std::string tr_address::readable() const
-{
-    auto buf = std::string{};
-    buf.reserve(INET6_ADDRSTRLEN);
-    this->readable(std::back_inserter(buf));
-    return buf;
 }
 
 template<typename OutputIt>
 OutputIt tr_address::readable(OutputIt out, tr_port port) const
 {
-    auto buf = std::array<char, INET6_ADDRSTRLEN>{};
+    auto addrbuf = std::array<char, TR_ADDRSTRLEN + 16>{};
-    tr_address_to_string_with_buf(this, std::data(buf), std::size(buf));
+    auto const addr_sv = readable(std::data(addrbuf), std::size(addrbuf), port);
-    return fmt::format_to(out, FMT_STRING("[{:s}]:{:d}"), std::data(buf), port.host());
+    return std::copy(std::begin(addr_sv), std::end(addr_sv), out);
 }
 
 template char* tr_address::readable<char*>(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;
 }

libtransmission/net.h

@@ -156,15 +156,20 @@ struct tr_address
     [[nodiscard]] static std::pair<tr_address, uint8_t const*> fromCompact6(uint8_t const* compact) noexcept;
 
     // human-readable formatting
-
     template<typename OutputIt>
-    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<typename OutputIt>
+    [[nodiscard]] constexpr auto isIPv4() const noexcept
-    OutputIt readable(OutputIt out, tr_port) const;
+    {
+        return type == TR_AF_INET;
+    }
 
-    [[nodiscard]] std::string readable() const;
+    [[nodiscard]] constexpr auto isIPv6() const noexcept
-    [[nodiscard]] std::string readable(tr_port) const;
+    {
+        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);

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;

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->upnpStatus = tr_upnpPulse(s->upnp, private_peer_port, is_enabled, do_check, session->bind_ipv4->addr.readable());
-        s->upnp,
-        private_peer_port,
-        is_enabled,
-        do_check,
-        tr_address_to_string(&session->bind_ipv4->addr));
 
     auto const new_status = tr_sharedTraversalStatus(s);
 

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_logAddTrace(fmt::format("new incoming connection {} ({})", clientSocket, clientAddr.readable(clientPort)));
-        tr_address_and_port_to_string(addrstr, sizeof(addrstr), &clientAddr, clientPort);
-        tr_logAddTrace(fmt::format("new incoming connection {} ({})", clientSocket, addrstr));
 
         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_ipv4, s->bind_ipv4->addr.readable());
-    tr_variantDictAddStr(d, TR_KEY_bind_address_ipv6, tr_address_to_string(&s->bind_ipv6->addr));
+    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));

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);
+    return tr_upnpDiscover(2000, bindaddr.c_str());
-    tr_free(bindaddr);
-    return ret;
 }
 
 template<typename T>
@@ -253,17 +251,17 @@ static bool isFutureReady(std::future<T> 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<UPNPDev*(char*)>{ discoverThreadfunc };
+        auto task = std::packaged_task<UPNPDev*(std::string)>{ 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 &&

libtransmission/upnp.h

@@ -14,6 +14,8 @@
  * @{
  */
 
+#include <string>
+
 #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);
 
 /* @} */