Skip to content

Commit

Permalink
Minor refactoring of net_addr_str function
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Robyt3 committed Jan 17, 2025
1 parent 619c36b commit aa10f07
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 34 deletions.
23 changes: 11 additions & 12 deletions src/base/system.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 3 additions & 5 deletions src/base/system.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
8 changes: 4 additions & 4 deletions src/engine/client/client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
Expand Down
6 changes: 1 addition & 5 deletions src/engine/client/serverbrowser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
23 changes: 15 additions & 8 deletions src/test/netaddr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}, "");
}

0 comments on commit aa10f07

Please sign in to comment.