From aa10f0792f29de9a0b5a38c2c961efe6a3b5caa5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=BCller?= Date: Fri, 17 Jan 2025 21:25:29 +0100 Subject: [PATCH] Minor refactoring of `net_addr_str` function Replace mostly unused `bool` return value with assertion to ensure the address has a valid type instead of outputting `unknown type` as the address. Also test the assertion failure by changing the existing test case to death tests. See https://google.github.io/googletest/advanced.html#death-tests for details. The death test names must end with `DeathTest` so GoogleTest runs these test cases before all other tests. Use `bool` instead of `int` for the `add_port` parameter. Minor improvement of doxygen comment. --- src/base/system.cpp | 23 +++++++++++------------ src/base/system.h | 8 +++----- src/engine/client/client.cpp | 8 ++++---- src/engine/client/serverbrowser.cpp | 6 +----- src/test/netaddr.cpp | 23 +++++++++++++++-------- 5 files changed, 34 insertions(+), 34 deletions(-) diff --git a/src/base/system.cpp b/src/base/system.cpp index 62d70859aee..35596d7339e 100644 --- a/src/base/system.cpp +++ b/src/base/system.cpp @@ -1144,36 +1144,35 @@ void net_addr_str_v6(const unsigned short ip[8], int port, char *buffer, int buf } } -bool net_addr_str(const NETADDR *addr, char *string, int max_length, int add_port) +void net_addr_str(const NETADDR *addr, char *string, int max_length, bool add_port) { if(addr->type & NETTYPE_IPV4 || addr->type & NETTYPE_WEBSOCKET_IPV4) { - if(add_port != 0) + if(add_port) + { str_format(string, max_length, "%d.%d.%d.%d:%d", addr->ip[0], addr->ip[1], addr->ip[2], addr->ip[3], addr->port); + } else + { str_format(string, max_length, "%d.%d.%d.%d", addr->ip[0], addr->ip[1], addr->ip[2], addr->ip[3]); + } } else if(addr->type & NETTYPE_IPV6) { - int port = -1; unsigned short ip[8]; - int i; - if(add_port) - { - port = addr->port; - } - for(i = 0; i < 8; i++) + for(int i = 0; i < 8; i++) { ip[i] = (addr->ip[i * 2] << 8) | (addr->ip[i * 2 + 1]); } + int port = add_port ? addr->port : -1; net_addr_str_v6(ip, port, string, max_length); } else { - str_format(string, max_length, "unknown type %d", addr->type); - return false; + char error[64]; + str_format(error, sizeof(error), "unknown NETADDR type %d", addr->type); + dbg_assert(false, error); } - return true; } static int priv_net_extract(const char *hostname, char *host, int max_host, int *port) diff --git a/src/base/system.h b/src/base/system.h index 39fb784ce6e..bc333bb2306 100644 --- a/src/base/system.h +++ b/src/base/system.h @@ -865,13 +865,11 @@ int net_addr_comp_noport(const NETADDR *a, const NETADDR *b); * @param addr Address to turn into a string. * @param string Buffer to fill with the string. * @param max_length Maximum size of the string. - * @param add_port add port to string or not + * @param add_port Whether or not to add the port to string. * - * @return true on success - * - * @remark The string will always be zero terminated + * @remark The string will always be zero terminated. */ -bool net_addr_str(const NETADDR *addr, char *string, int max_length, int add_port); +void net_addr_str(const NETADDR *addr, char *string, int max_length, bool add_port); /** * Turns url string into a network address struct. diff --git a/src/engine/client/client.cpp b/src/engine/client/client.cpp index 1f1cb3dd23d..a19b620d9b4 100644 --- a/src/engine/client/client.cpp +++ b/src/engine/client/client.cpp @@ -540,8 +540,7 @@ void CClient::Connect(const char *pAddress, const char *pPassword) Disconnect(); dbg_assert(m_State == IClient::STATE_OFFLINE, "Disconnect must ensure that client is offline"); - char aLastAddr[NETADDR_MAXSTRSIZE]; - net_addr_str(&ServerAddress(), aLastAddr, sizeof(aLastAddr), true); + const NETADDR LastAddr = ServerAddress(); if(pAddress != m_aConnectAddressStr) str_copy(m_aConnectAddressStr, pAddress); @@ -579,15 +578,16 @@ void CClient::Connect(const char *pAddress, const char *pPassword) { NextAddr.port = 8303; } - char aNextAddr[NETADDR_MAXSTRSIZE]; if(Sixup) NextAddr.type |= NETTYPE_TW7; else OnlySixup = false; + + char aNextAddr[NETADDR_MAXSTRSIZE]; net_addr_str(&NextAddr, aNextAddr, sizeof(aNextAddr), true); log_debug("client", "resolved connect address '%s' to %s", aBuffer, aNextAddr); - if(!str_comp(aNextAddr, aLastAddr)) + if(NextAddr == LastAddr) { m_SendPassword = true; } diff --git a/src/engine/client/serverbrowser.cpp b/src/engine/client/serverbrowser.cpp index 07cd529da78..76a1f555af7 100644 --- a/src/engine/client/serverbrowser.cpp +++ b/src/engine/client/serverbrowser.cpp @@ -702,11 +702,7 @@ void ServerBrowserFormatAddresses(char *pBuffer, int BufferSize, NETADDR *pAddrs return; } char aIpAddr[512]; - if(!net_addr_str(&pAddrs[i], aIpAddr, sizeof(aIpAddr), true)) - { - str_copy(pBuffer, aIpAddr, BufferSize); - return; - } + net_addr_str(&pAddrs[i], aIpAddr, sizeof(aIpAddr), true); if(pAddrs[i].type & NETTYPE_TW7) { str_format( diff --git a/src/test/netaddr.cpp b/src/test/netaddr.cpp index 7b199e39322..7a2b86047ba 100644 --- a/src/test/netaddr.cpp +++ b/src/test/netaddr.cpp @@ -134,13 +134,20 @@ TEST(NetAddr, FromStrInvalid) EXPECT_TRUE(net_addr_from_str(&Addr, "[::]:c")); } -TEST(NetAddr, StrInvalid) +TEST(NetAddrDeathTest, StrInvalid1) { - NETADDR Addr = {0}; - char aBuf1[NETADDR_MAXSTRSIZE]; - char aBuf2[NETADDR_MAXSTRSIZE]; - net_addr_str(&Addr, aBuf1, sizeof(aBuf1), true); - EXPECT_STREQ(aBuf1, "unknown type 0"); - net_addr_str(&Addr, aBuf2, sizeof(aBuf2), false); - EXPECT_STREQ(aBuf2, "unknown type 0"); + ASSERT_DEATH({ + NETADDR Addr = {0}; + char aBuf[NETADDR_MAXSTRSIZE]; + net_addr_str(&Addr, aBuf, sizeof(aBuf), true); + }, ""); +} + +TEST(NetAddrDeathTest, StrInvalid2) +{ + ASSERT_DEATH({ + NETADDR Addr = {0}; + char aBuf[NETADDR_MAXSTRSIZE]; + net_addr_str(&Addr, aBuf, sizeof(aBuf), false); + }, ""); }