From a4dd67ae45088e2496ffcca9833809f908d1bfd4 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Sun, 13 Sep 2020 16:43:29 -0500 Subject: [PATCH] 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 | 7 ++++++- libtransmission/log.c | 19 ++++++++----------- libtransmission/net.c | 19 ++++++++++++++----- libtransmission/net.h | 2 ++ libtransmission/peer-io.c | 25 ++++++++++++++++--------- libtransmission/peer-io.h | 4 +--- libtransmission/peer-mgr.c | 6 ++++-- libtransmission/peer-msgs.c | 16 ++++++---------- libtransmission/session.c | 4 +++- libtransmission/tr-macros.h | 2 ++ libtransmission/tr-utp.c | 3 --- libtransmission/utils.c | 22 +++++++++++----------- libtransmission/utils.h | 20 +++++++++----------- 13 files changed, 82 insertions(+), 67 deletions(-) diff --git a/libtransmission/handshake.c b/libtransmission/handshake.c index c6d57fc16..56fa9e1b1 100644 --- a/libtransmission/handshake.c +++ b/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) { diff --git a/libtransmission/log.c b/libtransmission/log.c index f65c69708..9809ebcb8 100644 --- a/libtransmission/log.c +++ b/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) { diff --git a/libtransmission/net.c b/libtransmission/net.c index 8fc8e87e0..a9816a924 100644 --- a/libtransmission/net.c +++ b/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 diff --git a/libtransmission/net.h b/libtransmission/net.h index 7b54ec314..8f2abdf7f 100644 --- a/libtransmission/net.h +++ b/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); diff --git a/libtransmission/peer-io.c b/libtransmission/peer-io.c index 86d4c5a3e..dc1028ae8 100644 --- a/libtransmission/peer-io.c +++ b/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) diff --git a/libtransmission/peer-io.h b/libtransmission/peer-io.h index 9a560e275..cd347e10f 100644 --- a/libtransmission/peer-io.h +++ b/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); diff --git a/libtransmission/peer-mgr.c b/libtransmission/peer-mgr.c index 536cc26cc..1c1a1e3e6 100644 --- a/libtransmission/peer-mgr.c +++ b/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 */ diff --git a/libtransmission/peer-msgs.c b/libtransmission/peer-msgs.c index 9442c8373..1637d7f3d 100644 --- a/libtransmission/peer-msgs.c +++ b/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; } diff --git a/libtransmission/session.c b/libtransmission/session.c index 91a5312dd..60c3dc140 100644 --- a/libtransmission/session.c +++ b/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)); diff --git a/libtransmission/tr-macros.h b/libtransmission/tr-macros.h index c174cf258..39b10485e 100644 --- a/libtransmission/tr-macros.h +++ b/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++ */ diff --git a/libtransmission/tr-utp.c b/libtransmission/tr-utp.c index 45bf14eb7..7c30ac69f 100644 --- a/libtransmission/tr-utp.c +++ b/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; } diff --git a/libtransmission/utils.c b/libtransmission/utils.c index 745f8c41a..0eaccbd12 100644 --- a/libtransmission/utils.c +++ b/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); } diff --git a/libtransmission/utils.h b/libtransmission/utils.h index 4936a2d8f..550123456 100644 --- a/libtransmission/utils.h +++ b/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);