cleanup: Use a struct for the ip_ntoa buffer.

Every use of this function needs to allocate the same buffer. None of
the callers uses a differently sized buffer, so we might as well put it
in a struct and have the type checker prove the buffer size is correct.

Also rename `ip_ntoa` to `net_ip_ntoa` to avoid clashes with ESP-IDF
system libraries which define this function as well.
This commit is contained in:
iphydf 2022-04-03 15:57:12 +00:00
parent e4d1958ffa
commit 7a3ead591f
No known key found for this signature in database
GPG Key ID: 3855DBA2D74403C9
8 changed files with 70 additions and 81 deletions

View File

@ -35,11 +35,11 @@ static void test_addr_resolv_localhost(void)
ck_assert_msg(res, "Resolver failed: %d, %s", error, strerror);
net_kill_strerror(strerror);
char ip_str[IP_NTOA_LEN];
Ip_Ntoa ip_str;
ck_assert_msg(net_family_is_ipv4(ip.family), "Expected family TOX_AF_INET, got %u.", ip.family.value);
const uint32_t loopback = get_ip4_loopback().uint32;
ck_assert_msg(ip.ip.v4.uint32 == loopback, "Expected 127.0.0.1, got %s.",
ip_ntoa(&ip, ip_str, sizeof(ip_str)));
net_ip_ntoa(&ip, &ip_str));
ip_init(&ip, 1); // ipv6enabled = 1
res = addr_resolve_or_parse_ip(ns, localhost, &ip, nullptr);
@ -62,7 +62,7 @@ static void test_addr_resolv_localhost(void)
ip.family.value);
IP6 ip6_loopback = get_ip6_loopback();
ck_assert_msg(!memcmp(&ip.ip.v6, &ip6_loopback, sizeof(IP6)), "Expected ::1, got %s.",
ip_ntoa(&ip, ip_str, sizeof(ip_str)));
net_ip_ntoa(&ip, &ip_str));
if (localhost_split) {
printf("Localhost seems to be split in two.\n");
@ -85,17 +85,17 @@ static void test_addr_resolv_localhost(void)
ck_assert_msg(net_family_is_ipv6(ip.family), "Expected family TOX_AF_INET6 (%d), got %u.", TOX_AF_INET6,
ip.family.value);
ck_assert_msg(!memcmp(&ip.ip.v6, &ip6_loopback, sizeof(IP6)), "Expected ::1, got %s.",
ip_ntoa(&ip, ip_str, sizeof(ip_str)));
net_ip_ntoa(&ip, &ip_str));
ck_assert_msg(net_family_is_ipv4(extra.family), "Expected family TOX_AF_INET (%d), got %u.", TOX_AF_INET,
extra.family.value);
ck_assert_msg(extra.ip.v4.uint32 == loopback, "Expected 127.0.0.1, got %s.",
ip_ntoa(&ip, ip_str, sizeof(ip_str)));
net_ip_ntoa(&ip, &ip_str));
#elif 0
// TODO(iphydf): Fix this to work on IPv6-supporting systems.
ck_assert_msg(net_family_is_ipv4(ip.family), "Expected family TOX_AF_INET (%d), got %u.", TOX_AF_INET, ip.family.value);
ck_assert_msg(ip.ip.v4.uint32 == loopback, "Expected 127.0.0.1, got %s.",
ip_ntoa(&ip, ip_str, sizeof(ip_str)));
net_ip_ntoa(&ip, &ip_str));
#endif
}

View File

@ -1 +1 @@
4a2bfe7abf6060f252154062818541e485344b350cf975f7aeea78ff249bfa65 /usr/local/bin/tox-bootstrapd
f3781691aed256efccda556f5663a4ab95c460aa8c1d665667c8b80c44e4d630 /usr/local/bin/tox-bootstrapd

View File

@ -482,10 +482,10 @@ int pack_ip_port(const Logger *logger, uint8_t *data, uint16_t length, const IP_
is_ipv4 = false;
family = TOX_TCP_INET6;
} else {
char ip_str[IP_NTOA_LEN];
Ip_Ntoa ip_str;
// TODO(iphydf): Find out why we're trying to pack invalid IPs, stop
// doing that, and turn this into an error.
LOGGER_TRACE(logger, "cannot pack invalid IP: %s", ip_ntoa(&ip_port->ip, ip_str, sizeof(ip_str)));
LOGGER_TRACE(logger, "cannot pack invalid IP: %s", net_ip_ntoa(&ip_port->ip, &ip_str));
return -1;
}
@ -781,12 +781,13 @@ static void update_client(const Logger *log, const Mono_Time *mono_time, int ind
}
if (!ipport_equal(&assoc->ip_port, ip_port)) {
char ip_str[IP_NTOA_LEN];
Ip_Ntoa ip_str_from;
Ip_Ntoa ip_str_to;
LOGGER_TRACE(log, "coipil[%u]: switching ipv%d from %s:%u to %s:%u",
index, ip_version,
ip_ntoa(&assoc->ip_port.ip, ip_str, sizeof(ip_str)),
net_ip_ntoa(&assoc->ip_port.ip, &ip_str_from),
net_ntohs(assoc->ip_port.port),
ip_ntoa(&ip_port->ip, ip_str, sizeof(ip_str)),
net_ip_ntoa(&ip_port->ip, &ip_str_to),
net_ntohs(ip_port->port));
}

View File

@ -2419,10 +2419,10 @@ void do_messenger(Messenger *m, void *userdata)
last_pinged = 999;
}
char ip_str[IP_NTOA_LEN];
Ip_Ntoa ip_str;
char id_str[IDSTRING_LEN];
LOGGER_TRACE(m->log, "C[%2u] %s:%u [%3u] %s",
client, ip_ntoa(&assoc->ip_port.ip, ip_str, sizeof(ip_str)),
client, net_ip_ntoa(&assoc->ip_port.ip, &ip_str),
net_ntohs(assoc->ip_port.port), last_pinged,
id_to_string(cptr->public_key, id_str, sizeof(id_str)));
}
@ -2492,10 +2492,10 @@ void do_messenger(Messenger *m, void *userdata)
last_pinged = 999;
}
char ip_str[IP_NTOA_LEN];
Ip_Ntoa ip_str;
char id_str[IDSTRING_LEN];
LOGGER_TRACE(m->log, "F[%2u] => C[%2u] %s:%u [%3u] %s",
friend_idx, client, ip_ntoa(&assoc->ip_port.ip, ip_str, sizeof(ip_str)),
friend_idx, client, net_ip_ntoa(&assoc->ip_port.ip, &ip_str),
net_ntohs(assoc->ip_port.port), last_pinged,
id_to_string(cptr->public_key, id_str, sizeof(id_str)));
}

View File

@ -670,25 +670,25 @@ static void loglogdata(const Logger *log, const char *message, const uint8_t *bu
uint16_t buflen, const IP_Port *ip_port, long res)
{
if (res < 0) { /* Windows doesn't necessarily know `%zu` */
char ip_str[IP_NTOA_LEN];
Ip_Ntoa ip_str;
const int error = net_error();
char *strerror = net_new_strerror(error);
LOGGER_TRACE(log, "[%2u] %s %3u%c %s:%u (%u: %s) | %08x%08x...%02x",
buffer[0], message, min_u16(buflen, 999), 'E',
ip_ntoa(&ip_port->ip, ip_str, sizeof(ip_str)), net_ntohs(ip_port->port), error,
net_ip_ntoa(&ip_port->ip, &ip_str), net_ntohs(ip_port->port), error,
strerror, data_0(buflen, buffer), data_1(buflen, buffer), buffer[buflen - 1]);
net_kill_strerror(strerror);
} else if ((res > 0) && ((size_t)res <= buflen)) {
char ip_str[IP_NTOA_LEN];
Ip_Ntoa ip_str;
LOGGER_TRACE(log, "[%2u] %s %3u%c %s:%u (%u: %s) | %08x%08x...%02x",
buffer[0], message, min_u16(res, 999), (size_t)res < buflen ? '<' : '=',
ip_ntoa(&ip_port->ip, ip_str, sizeof(ip_str)), net_ntohs(ip_port->port), 0, "OK",
net_ip_ntoa(&ip_port->ip, &ip_str), net_ntohs(ip_port->port), 0, "OK",
data_0(buflen, buffer), data_1(buflen, buffer), buffer[buflen - 1]);
} else { /* empty or overwrite */
char ip_str[IP_NTOA_LEN];
Ip_Ntoa ip_str;
LOGGER_TRACE(log, "[%2u] %s %lu%c%u %s:%u (%u: %s) | %08x%08x...%02x",
buffer[0], message, res, res == 0 ? '!' : '>', buflen,
ip_ntoa(&ip_port->ip, ip_str, sizeof(ip_str)), net_ntohs(ip_port->port), 0, "OK",
net_ip_ntoa(&ip_port->ip, &ip_str), net_ntohs(ip_port->port), 0, "OK",
data_0(buflen, buffer), data_1(buflen, buffer), buffer[buflen - 1]);
}
}
@ -1198,8 +1198,8 @@ Networking_Core *new_networking_ex(
if (res == 0) {
temp->port = *portptr;
char ip_str[IP_NTOA_LEN];
LOGGER_DEBUG(log, "Bound successfully to %s:%u", ip_ntoa(ip, ip_str, sizeof(ip_str)),
Ip_Ntoa ip_str;
LOGGER_DEBUG(log, "Bound successfully to %s:%u", net_ip_ntoa(ip, &ip_str),
net_ntohs(temp->port));
/* errno isn't reset on success, only set on failure, the failed
@ -1225,11 +1225,11 @@ Networking_Core *new_networking_ex(
*portptr = net_htons(port_to_try);
}
char ip_str[IP_NTOA_LEN];
Ip_Ntoa ip_str;
int neterror = net_error();
char *strerror = net_new_strerror(neterror);
LOGGER_ERROR(log, "failed to bind socket: %d, %s IP: %s port_from: %u port_to: %u", neterror, strerror,
ip_ntoa(ip, ip_str, sizeof(ip_str)), port_from, port_to);
net_ip_ntoa(ip, &ip_str), port_from, port_to);
net_kill_strerror(strerror);
kill_networking(temp);
@ -1402,34 +1402,31 @@ void ipport_copy(IP_Port *target, const IP_Port *source)
*target = *source;
}
/** @brief converts ip into a string
/** @brief Converts IP into a string.
*
* @param ip_str must be of length at least IP_NTOA_LEN
* Writes error message into the buffer on error.
*
* writes error message into the buffer on error
* @param ip_str contains a buffer of the required size.
*
* @return ip_str
* @return Pointer to the buffer inside `ip_str` containing the IP string.
*/
const char *ip_ntoa(const IP *ip, char *ip_str, size_t length)
const char *net_ip_ntoa(const IP *ip, Ip_Ntoa *ip_str)
{
if (length < IP_NTOA_LEN) {
snprintf(ip_str, length, "Bad buf length");
return ip_str;
}
assert(ip_str != nullptr);
if (ip == nullptr) {
snprintf(ip_str, length, "(IP invalid: NULL)");
return ip_str;
snprintf(ip_str->buf, sizeof(ip_str->buf), "(IP invalid: NULL)");
return ip_str->buf;
}
if (!ip_parse_addr(ip, ip_str, length)) {
snprintf(ip_str, length, "(IP invalid, family %u)", ip->family.value);
return ip_str;
if (!ip_parse_addr(ip, ip_str->buf, sizeof(ip_str->buf))) {
snprintf(ip_str->buf, sizeof(ip_str->buf), "(IP invalid, family %u)", ip->family.value);
return ip_str->buf;
}
/* brute force protection against lacking termination */
ip_str[length - 1] = '\0';
return ip_str;
ip_str->buf[sizeof(ip_str->buf) - 1] = '\0';
return ip_str->buf;
}
bool ip_parse_addr(const IP *ip, char *address, size_t length)
@ -1611,8 +1608,6 @@ bool net_connect(const Logger *log, Socket sock, const IP_Port *ip_port)
{
struct sockaddr_storage addr = {0};
size_t addrsize;
char ip_str[IP_NTOA_LEN];
ip_ntoa(&ip_port->ip, ip_str, sizeof(ip_str));
if (net_family_is_ipv4(ip_port->ip.family)) {
struct sockaddr_in *addr4 = (struct sockaddr_in *)&addr;
@ -1629,15 +1624,18 @@ bool net_connect(const Logger *log, Socket sock, const IP_Port *ip_port)
fill_addr6(&ip_port->ip.ip.v6, &addr6->sin6_addr);
addr6->sin6_port = ip_port->port;
} else {
LOGGER_ERROR(log, "cannot connect to %s:%d which is neither IPv4 nor IPv6", ip_str, ip_port->port);
Ip_Ntoa ip_str;
LOGGER_ERROR(log, "cannot connect to %s:%d which is neither IPv4 nor IPv6",
net_ip_ntoa(&ip_port->ip, &ip_str), ip_port->port);
return false;
}
#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
return addrsize != 0;
#else
Ip_Ntoa ip_str;
LOGGER_DEBUG(log, "connecting socket %d to %s:%d",
(int)sock.sock, ip_ntoa(&ip_port->ip, ip_str, sizeof(ip_str)), net_ntohs(ip_port->port));
(int)sock.sock, net_ip_ntoa(&ip_port->ip, &ip_str), net_ntohs(ip_port->port));
errno = 0;
if (connect(sock.sock, (struct sockaddr *)&addr, addrsize) == -1) {
@ -1646,7 +1644,8 @@ bool net_connect(const Logger *log, Socket sock, const IP_Port *ip_port)
// Non-blocking socket: "Operation in progress" means it's connecting.
if (!should_ignore_connect_error(error)) {
char *net_strerror = net_new_strerror(error);
LOGGER_ERROR(log, "failed to connect to %s:%d: %d (%s)", ip_str, ip_port->port, error, net_strerror);
LOGGER_ERROR(log, "failed to connect to %s:%d: %d (%s)",
ip_str.buf, ip_port->port, error, net_strerror);
net_kill_strerror(net_strerror);
return false;
}

View File

@ -328,16 +328,21 @@ bool ipv6_ipv4_in_v6(const IP6 *a);
/** this would be TOX_INET6_ADDRSTRLEN, but it might be too short for the error message */
#define IP_NTOA_LEN 96 // TODO(irungentoo): magic number. Why not INET6_ADDRSTRLEN ?
/** @brief converts ip into a string
typedef struct Ip_Ntoa {
char buf[IP_NTOA_LEN];
} Ip_Ntoa;
/** @brief Converts IP into a string.
*
* @param ip_str must be of length at least IP_NTOA_LEN
* Writes error message into the buffer on error.
*
* writes error message into the buffer on error
* @param ip_str contains a buffer of the required size.
*
* @return ip_str
* @return Pointer to the buffer inside `ip_str` containing the IP string.
*/
non_null()
const char *ip_ntoa(const IP *ip, char *ip_str, size_t length);
const char *net_ip_ntoa(const IP *ip, Ip_Ntoa *ip_str);
/**
* Parses IP structure into an address string.

View File

@ -6,48 +6,33 @@ namespace {
TEST(IpNtoa, DoesntWriteOutOfBounds)
{
char ip_str[IP_NTOA_LEN];
Ip_Ntoa ip_str;
IP ip;
ip.family = net_family_ipv6;
ip.ip.v6.uint64[0] = -1;
ip.ip.v6.uint64[1] = -1;
ip_ntoa(&ip, ip_str, sizeof(ip_str));
net_ip_ntoa(&ip, &ip_str);
EXPECT_EQ(std::string(ip_str), "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff");
EXPECT_LT(std::string(ip_str).length(), IP_NTOA_LEN);
}
TEST(IpNtoa, DoesntOverrunSmallBuffer)
{
// We have to try really hard to trick the compiler here not to realise that
// 10 is too small a buffer for the snprintf inside ip_ntoa and error out.
std::istringstream ss("10");
size_t len;
ss >> len;
std::vector<char> ip_str(len);
IP *ip = nullptr;
ip_ntoa(ip, ip_str.data(), ip_str.size());
EXPECT_EQ(std::string(ip_str.data()), "Bad buf l");
EXPECT_EQ(std::string(ip_str.buf), "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff");
EXPECT_LT(std::string(ip_str.buf).length(), IP_NTOA_LEN);
}
TEST(IpNtoa, ReportsInvalidIpFamily)
{
char ip_str[IP_NTOA_LEN];
Ip_Ntoa ip_str;
IP ip;
ip.family.value = 255 - net_family_ipv6.value;
ip.ip.v4.uint32 = 0;
ip_ntoa(&ip, ip_str, sizeof(ip_str));
net_ip_ntoa(&ip, &ip_str);
EXPECT_EQ(std::string(ip_str), "(IP invalid, family 245)");
EXPECT_EQ(std::string(ip_str.buf), "(IP invalid, family 245)");
}
TEST(IpNtoa, FormatsIPv4)
{
char ip_str[IP_NTOA_LEN];
Ip_Ntoa ip_str;
IP ip;
ip.family = net_family_ipv4;
ip.ip.v4.uint8[0] = 192;
@ -55,9 +40,9 @@ TEST(IpNtoa, FormatsIPv4)
ip.ip.v4.uint8[2] = 0;
ip.ip.v4.uint8[3] = 13;
ip_ntoa(&ip, ip_str, sizeof(ip_str));
net_ip_ntoa(&ip, &ip_str);
EXPECT_EQ(std::string(ip_str), "192.168.0.13");
EXPECT_EQ(std::string(ip_str.buf), "192.168.0.13");
}
TEST(IpParseAddr, FormatsIPv4)

View File

@ -315,10 +315,9 @@ static void tox_dht_get_nodes_response_handler(const DHT *dht, const Node_format
return;
}
char ip[IP_NTOA_LEN];
ip_ntoa(&node->ip_port.ip, ip, sizeof(ip));
tox_data->tox->dht_get_nodes_response_callback(tox_data->tox, node->public_key, ip, net_ntohs(node->ip_port.port),
Ip_Ntoa ip_str;
tox_data->tox->dht_get_nodes_response_callback(
tox_data->tox, node->public_key, net_ip_ntoa(&node->ip_port.ip, &ip_str), net_ntohs(node->ip_port.port),
tox_data->user_data);
}