chore: fix some fixmes (#1449)

* chore: remove assertions that were flagged for removal

* chore: remove unneeded check

* refactor: move tr_peerIoAddrStr to net.h

* refactor: formatter API now takes size_t args

* chore: remove out-of-date FIXME comment

* refactor: cleanup logging win32 ifdefs

libtransmission/handshake.c

@@ -131,7 +131,12 @@ struct tr_handshake
 ***
 **/
 
-#define dbgmsg(handshake, ...) tr_logAddDeepNamed(tr_peerIoGetAddrStr((handshake)->io), __VA_ARGS__)
+#define dbgmsg(handshake, ...) \
+    do { \
+        char addrstr[TR_ADDRSTRLEN]; \
+        tr_peerIoGetAddrStr(handshake->io, addrstr, sizeof(addrstr)); \
+        tr_logAddDeepNamed(addrstr, __VA_ARGS__); \
+    } while(0)
 
 static char const* getStateName(handshake_state_t const state)
 {

libtransmission/log.c

@@ -33,11 +33,6 @@ static inline bool IsDebuggerPresent(void)
     return false;
 }
 
-static inline void OutputDebugStringA(void const* data)
-{
-    TR_UNUSED(data);
-}
-
 #endif
 
 /***
@@ -175,13 +170,10 @@ void tr_logAddDeep(char const* file, int line, char const* name, char const* fmt
 
     if (fp != TR_BAD_SYS_FILE || IsDebuggerPresent())
     {
-        va_list args;
-        char timestr[64];
-        char* message;
-        size_t message_len;
         struct evbuffer* buf = evbuffer_new();
         char* base = tr_sys_path_basename(file, NULL);
 
+        char timestr[64];
         evbuffer_add_printf(buf, "[%s] ", tr_logGetTimeStr(timestr, sizeof(timestr)));
 
         if (name != NULL)
@@ -189,13 +181,18 @@ void tr_logAddDeep(char const* file, int line, char const* name, char const* fmt
             evbuffer_add_printf(buf, "%s ", name);
         }
 
+        va_list args;
         va_start(args, fmt);
         evbuffer_add_vprintf(buf, fmt, args);
         va_end(args);
         evbuffer_add_printf(buf, " (%s:%d)" TR_NATIVE_EOL_STR, base, line);
-        /* FIXME (libevent2) ifdef this out for nonwindows platforms */
-        message = evbuffer_free_to_str(buf, &message_len);
+
+        size_t message_len = 0;
+        char* const message = evbuffer_free_to_str(buf, &message_len);
+
+#ifdef _WIN32
         OutputDebugStringA(message);
+#endif
 
         if (fp != TR_BAD_SYS_FILE)
         {

libtransmission/net.c

@@ -41,7 +41,7 @@
 #include "fdlimit.h" /* tr_fdSocketClose() */
 #include "log.h"
 #include "net.h"
-#include "peer-io.h" /* tr_peerIoAddrStr() FIXME this should be moved to net.h */
+#include "peer-socket.h" /* for struct tr_peer_socket */
 #include "session.h" /* tr_sessionGetPublicAddress() */
 #include "tr-assert.h"
 #include "tr-macros.h"
@@ -86,6 +86,14 @@ char* tr_net_strerror(char* buf, size_t buflen, int err)
     return buf;
 }
 
+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));
+    tr_snprintf(buf, buflen, "[%s]:%u", addr_buf, ntohs(port));
+    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));
@@ -350,8 +358,9 @@ struct tr_peer_socket tr_netOpenPeerSocket(tr_session* session, tr_address const
 
     if (tr_logGetDeepEnabled())
     {
-        tr_logAddDeep(__FILE__, __LINE__, NULL, "New OUTGOING connection %" PRIdMAX " (%s)", (intmax_t)s,
-            tr_peerIoAddrStr(addr, port));
+        char addrstr[TR_ADDRSTRLEN];
+        tr_address_and_port_to_string(addrstr, sizeof(addrstr), addr, port);
+        tr_logAddDeep(__FILE__, __LINE__, NULL, "New OUTGOING connection %" PRIdMAX " (%s)", (intmax_t)s, addrstr);
     }
 
     return ret;
@@ -404,8 +413,8 @@ static tr_socket_t tr_netBindTCPImpl(tr_address const* addr, tr_port port, bool
     }
 
     optval = 1;
-    setsockopt(fd, SOL_SOCKET, SO_KEEPALIVE, (void const*)&optval, sizeof(optval));
-    setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, (void const*)&optval, sizeof(optval));
+    (void)setsockopt(fd, SOL_SOCKET, SO_KEEPALIVE, (void const*)&optval, sizeof(optval));
+    (void)setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, (void const*)&optval, sizeof(optval));
 
 #ifdef IPV6_V6ONLY
 

libtransmission/net.h

@@ -105,6 +105,8 @@ 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_sockaddr_storage(tr_address* setme, tr_port* port, struct sockaddr_storage const* src);

libtransmission/peer-io.c

@@ -70,7 +70,12 @@ static size_t guessPacketOverhead(size_t d)
 ***
 **/
 
-#define dbgmsg(io, ...) tr_logAddDeepNamed(tr_peerIoGetAddrStr(io), __VA_ARGS__)
+#define dbgmsg(io, ...) \
+    do { \
+        char addrstr[TR_ADDRSTRLEN]; \
+        tr_peerIoGetAddrStr(io, addrstr, sizeof(addrstr)); \
+        tr_logAddDeepNamed(addrstr, __VA_ARGS__); \
+    } while(0)
 
 /**
 ***
@@ -966,16 +971,18 @@ tr_address const* tr_peerIoGetAddress(tr_peerIo const* io, tr_port* port)
     return &io->addr;
 }
 
-char const* tr_peerIoAddrStr(tr_address const* addr, tr_port port)
+char const* tr_peerIoGetAddrStr(tr_peerIo const* io, char* buf, size_t buflen)
 {
-    static char buf[512];
-    tr_snprintf(buf, sizeof(buf), "[%s]:%u", tr_address_to_string(addr), ntohs(port));
-    return buf;
-}
+    if (tr_isPeerIo(io))
+    {
+        tr_address_and_port_to_string(buf, buflen, &io->addr, io->port);
+    }
+    else
+    {
+        tr_strlcpy(buf, "error", buflen);
+    }
 
-char const* tr_peerIoGetAddrStr(tr_peerIo const* io)
-{
-    return tr_isPeerIo(io) ? tr_peerIoAddrStr(&io->addr, io->port) : "error";
+    return buf;
 }
 
 void tr_peerIoSetIOFuncs(tr_peerIo* io, tr_can_read_cb readcb, tr_did_write_cb writecb, tr_net_error_cb errcb, void* userData)

libtransmission/peer-io.h

@@ -182,9 +182,7 @@ static inline tr_session* tr_peerIoGetSession(tr_peerIo* io)
     return io->session;
 }
 
-char const* tr_peerIoAddrStr(struct tr_address const* addr, tr_port port);
-
-char const* tr_peerIoGetAddrStr(tr_peerIo const* io);
+char const* tr_peerIoGetAddrStr(tr_peerIo const* io, char* buf, size_t buflen);
 
 struct tr_address const* tr_peerIoGetAddress(tr_peerIo const* io, tr_port* port);
 

libtransmission/peer-mgr.c

@@ -155,7 +155,10 @@ static bool tr_isAtom(struct peer_atom const* atom)
 
 static char const* tr_atomAddrStr(struct peer_atom const* atom)
 {
-    return atom != NULL ? tr_peerIoAddrStr(&atom->addr, atom->port) : "[no atom]";
+    static char addrstr[TR_ADDRSTRLEN];
+    return atom != NULL ?
+        tr_address_and_port_to_string(addrstr, sizeof(addrstr), &atom->addr, atom->port) :
+        "[no atom]";
 }
 
 struct block_request
@@ -3866,7 +3869,6 @@ static void bandwidthPulse(evutil_socket_t fd, short what, void* vmgr)
     tr_session* session = mgr->session;
     managerLock(mgr);
 
-    /* FIXME: this next line probably isn't necessary... */
     pumpAllPeers(mgr);
 
     /* allocate bandwidth to the peers */

libtransmission/peer-msgs.c

@@ -267,12 +267,16 @@ static void myDebug(char const* file, int line, struct tr_peerMsgs const* msgs,
     {
         va_list args;
         char timestr[64];
+        char addrstr[TR_ADDRSTRLEN];
         struct evbuffer* buf = evbuffer_new();
         char* base = tr_sys_path_basename(file, NULL);
         char* message;
 
-        evbuffer_add_printf(buf, "[%s] %s - %s [%s]: ", tr_logGetTimeStr(timestr, sizeof(timestr)), tr_torrentName(
-            msgs->torrent), tr_peerIoGetAddrStr(msgs->io), tr_quark_get_string(msgs->peer.client, NULL));
+        evbuffer_add_printf(buf, "[%s] %s - %s [%s]: ",
+            tr_logGetTimeStr(timestr, sizeof(timestr)),
+            tr_torrentName(msgs->torrent),
+            tr_peerIoGetAddrStr(msgs->io, addrstr, sizeof(addrstr)),
+            tr_quark_get_string(msgs->peer.client, NULL));
         va_start(args, fmt);
         evbuffer_add_vprintf(buf, fmt, args);
         va_end(args);
@@ -660,13 +664,6 @@ static bool tr_peerMsgsCalculateActive(tr_peerMsgs const* msgs, tr_direction dir
     if (direction == TR_CLIENT_TO_PEER)
     {
         is_active = tr_peerMsgsIsPeerInterested(msgs) && !tr_peerMsgsIsPeerChoked(msgs);
-
-        /* FIXME: https://trac.transmissionbt.com/ticket/5505
-        if (is_active)
-        {
-            TR_ASSERT(!tr_peerIsSeed(&msgs->peer));
-        }
-        */
     }
     else /* TR_PEER_TO_CLIENT */
     {
@@ -2706,7 +2703,6 @@ bool tr_peerMsgsIsIncomingConnection(tr_peerMsgs const* msgs)
 
 bool tr_isPeerMsgs(void const* msgs)
 {
-    /* FIXME: this is pretty crude */
     return msgs != NULL && ((struct tr_peerMsgs*)msgs)->magic_number == MAGIC_NUMBER;
 }
 

libtransmission/session.c

@@ -178,8 +178,10 @@ static void accept_incoming_peer(evutil_socket_t fd, short what, void* vsession)
     {
         if (tr_logGetDeepEnabled())
         {
+            char addrstr[TR_ADDRSTRLEN];
+            tr_address_and_port_to_string(addrstr, sizeof(addrstr), &clientAddr, clientPort);
             tr_logAddDeep(__FILE__, __LINE__, NULL, "new incoming connection %" PRIdMAX " (%s)", (intmax_t)clientSocket,
-                tr_peerIoAddrStr(&clientAddr, clientPort));
+                addrstr);
         }
 
         tr_peerMgrAddIncoming(session->peerMgr, &clientAddr, clientPort, tr_peer_socket_tcp_create(clientSocket));

libtransmission/tr-macros.h

@@ -164,6 +164,8 @@
 
 #define TR_INET6_ADDRSTRLEN 46
 
+#define TR_ADDRSTRLEN 64
+
 #define TR_BAD_SIZE ((size_t)-1)
 
 /* Guard C code in headers, while including them from C++ */

libtransmission/tr-utp.c

@@ -47,21 +47,18 @@ void UTP_Close(struct UTPSocket* socket)
 {
     tr_logAddNamedError(MY_NAME, "UTP_Close(%p) was called.", socket);
     dbgmsg("UTP_Close(%p) was called.", socket);
-    TR_ASSERT(false); /* FIXME: this is too much for the long term, but probably needed in the short term */
 }
 
 void UTP_RBDrained(struct UTPSocket* socket)
 {
     tr_logAddNamedError(MY_NAME, "UTP_RBDrained(%p) was called.", socket);
     dbgmsg("UTP_RBDrained(%p) was called.", socket);
-    TR_ASSERT(false); /* FIXME: this is too much for the long term, but probably needed in the short term */
 }
 
 bool UTP_Write(struct UTPSocket* socket, size_t count)
 {
     tr_logAddNamedError(MY_NAME, "UTP_RBDrained(%p, %zu) was called.", socket, count);
     dbgmsg("UTP_RBDrained(%p, %zu) was called.", socket, count);
-    TR_ASSERT(false); /* FIXME: this is too much for the long term, but probably needed in the short term */
     return false;
 }
 

libtransmission/utils.c

@@ -1959,7 +1959,7 @@ uint64_t tr_ntohll(uint64_t x)
 struct formatter_unit
 {
     char* name;
-    int64_t value;
+    size_t value;
 };
 
 struct formatter_units
@@ -1975,10 +1975,10 @@ enum
     TR_FMT_TB
 };
 
-static void formatter_init(struct formatter_units* units, unsigned int kilo, char const* kb, char const* mb, char const* gb,
+static void formatter_init(struct formatter_units* units, size_t kilo, char const* kb, char const* mb, char const* gb,
     char const* tb)
 {
-    uint64_t value;
+    size_t value;
 
     value = kilo;
     units->units[TR_FMT_KB].name = tr_strdup(kb);
@@ -1997,7 +1997,7 @@ static void formatter_init(struct formatter_units* units, unsigned int kilo, cha
     units->units[TR_FMT_TB].value = value;
 }
 
-static char* formatter_get_size_str(struct formatter_units const* u, char* buf, int64_t bytes, size_t buflen)
+static char* formatter_get_size_str(struct formatter_units const* u, char* buf, size_t bytes, size_t buflen)
 {
     int precision;
     double value;
@@ -2043,21 +2043,21 @@ static char* formatter_get_size_str(struct formatter_units const* u, char* buf,
 
 static struct formatter_units size_units;
 
-void tr_formatter_size_init(unsigned int kilo, char const* kb, char const* mb, char const* gb, char const* tb)
+void tr_formatter_size_init(size_t kilo, char const* kb, char const* mb, char const* gb, char const* tb)
 {
     formatter_init(&size_units, kilo, kb, mb, gb, tb);
 }
 
-char* tr_formatter_size_B(char* buf, int64_t bytes, size_t buflen)
+char* tr_formatter_size_B(char* buf, size_t bytes, size_t buflen)
 {
     return formatter_get_size_str(&size_units, buf, bytes, buflen);
 }
 
 static struct formatter_units speed_units;
 
-unsigned int tr_speed_K = 0U;
+size_t tr_speed_K = 0;
 
-void tr_formatter_speed_init(unsigned int kilo, char const* kb, char const* mb, char const* gb, char const* tb)
+void tr_formatter_speed_init(size_t kilo, char const* kb, char const* mb, char const* gb, char const* tb)
 {
     tr_speed_K = kilo;
     formatter_init(&speed_units, kilo, kb, mb, gb, tb);
@@ -2095,15 +2095,15 @@ char* tr_formatter_speed_KBps(char* buf, double KBps, size_t buflen)
 
 static struct formatter_units mem_units;
 
-unsigned int tr_mem_K = 0U;
+size_t tr_mem_K = 0;
 
-void tr_formatter_mem_init(unsigned int kilo, char const* kb, char const* mb, char const* gb, char const* tb)
+void tr_formatter_mem_init(size_t kilo, char const* kb, char const* mb, char const* gb, char const* tb)
 {
     tr_mem_K = kilo;
     formatter_init(&mem_units, kilo, kb, mb, gb, tb);
 }
 
-char* tr_formatter_mem_B(char* buf, int64_t bytes_per_second, size_t buflen)
+char* tr_formatter_mem_B(char* buf, size_t bytes_per_second, size_t buflen)
 {
     return formatter_get_size_str(&mem_units, buf, bytes_per_second, buflen);
 }

libtransmission/utils.h

@@ -378,32 +378,30 @@ uint64_t tr_ntohll(uint64_t);
 
 /* example: tr_formatter_size_init(1024, _("KiB"), _("MiB"), _("GiB"), _("TiB")); */
 
-void tr_formatter_size_init(unsigned int kilo, char const* kb, char const* mb, char const* gb, char const* tb);
+void tr_formatter_size_init(size_t kilo, char const* kb, char const* mb, char const* gb, char const* tb);
 
-void tr_formatter_speed_init(unsigned int kilo, char const* kb, char const* mb, char const* gb, char const* tb);
+void tr_formatter_speed_init(size_t kilo, char const* kb, char const* mb, char const* gb, char const* tb);
 
-void tr_formatter_mem_init(unsigned int kilo, char const* kb, char const* mb, char const* gb, char const* tb);
+void tr_formatter_mem_init(size_t kilo, char const* kb, char const* mb, char const* gb, char const* tb);
 
-extern unsigned int tr_speed_K;
-extern unsigned int tr_mem_K;
-extern unsigned int tr_size_K;
+extern size_t tr_speed_K;
+extern size_t tr_mem_K;
+extern size_t tr_size_K;
 
 /* format a speed from KBps into a user-readable string. */
 char* tr_formatter_speed_KBps(char* buf, double KBps, size_t buflen);
 
-// FIXME(ckerr): bytes should be a size_t
 /* format a memory size from bytes into a user-readable string. */
-char* tr_formatter_mem_B(char* buf, int64_t bytes, size_t buflen);
+char* tr_formatter_mem_B(char* buf, size_t bytes, size_t buflen);
 
 /* format a memory size from MB into a user-readable string. */
 static inline char* tr_formatter_mem_MB(char* buf, double MBps, size_t buflen)
 {
-    return tr_formatter_mem_B(buf, (int64_t)(MBps * tr_mem_K * tr_mem_K), buflen);
+    return tr_formatter_mem_B(buf, (size_t)(MBps * tr_mem_K * tr_mem_K), buflen);
 }
 
-// FIXME(ckerr): bytes should be a size_t
 /* format a file size from bytes into a user-readable string. */
-char* tr_formatter_size_B(char* buf, int64_t bytes, size_t buflen);
+char* tr_formatter_size_B(char* buf, size_t bytes, size_t buflen);
 
 void tr_formatter_get_units(void* dict);