Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Use a struct for the ip_ntoa buffer. #2248

Merged
merged 1 commit into from
Apr 3, 2022
Merged

refactor: Use a struct for the ip_ntoa buffer. #2248

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions auto_tests/network_test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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");
Expand All @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion other/bootstrap_daemon/docker/tox-bootstrapd.sha256
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
4a2bfe7abf6060f252154062818541e485344b350cf975f7aeea78ff249bfa65 /usr/local/bin/tox-bootstrapd
f3781691aed256efccda556f5663a4ab95c460aa8c1d665667c8b80c44e4d630 /usr/local/bin/tox-bootstrapd
11 changes: 6 additions & 5 deletions toxcore/DHT.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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));
}

Expand Down
8 changes: 4 additions & 4 deletions toxcore/Messenger.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)));
}
Expand Down Expand Up @@ -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)));
}
Expand Down
61 changes: 30 additions & 31 deletions toxcore/network.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
}
}
Expand Down Expand Up @@ -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
Expand All @@ -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);

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;
Expand All @@ -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) {
Expand All @@ -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;
}
Expand Down
15 changes: 10 additions & 5 deletions toxcore/network.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
35 changes: 10 additions & 25 deletions toxcore/network_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,58 +6,43 @@ 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));

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());
net_ip_ntoa(&ip, &ip_str);

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;
ip.ip.v4.uint8[1] = 168;
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)
Expand Down
7 changes: 3 additions & 4 deletions toxcore/tox.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down