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.

Fix assertion failure in client due to previous server address being invalid when connecting to the first server by comparing addresses directly instead of converting the invalid address to a string first.

Use `bool` instead of `int` for the `add_port` parameter.

Minor improvement of doxygen comment.

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)

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 the 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.

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;
 		}

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(

src/test/netaddr.cpp

@@ -134,13 +134,22 @@ 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);
+	},
+		"");
 }

src/tools/stun.cpp

@@ -83,7 +83,7 @@ int main(int argc, const char **argv)
 		if(Success)
 		{
 			char aStunAddr[NETADDR_MAXSTRSIZE];
-			net_addr_str(&StunAddr, aStunAddr, sizeof(aStunAddr), 1);
+			net_addr_str(&StunAddr, aStunAddr, sizeof(aStunAddr), true);
 			log_info("stun", "public ip address: %s", aStunAddr);
 			break;
 		}