From 7d399cedcfd20f0d91a8caf386ae3c63f4dcf285 Mon Sep 17 00:00:00 2001 From: Maxim Biro Date: Sat, 7 Apr 2018 22:09:42 -0400 Subject: [PATCH] Improve network error reporting on Windows Windows doesn't report network errors though errno, it has its own facilities. --- auto_tests/network_test.c | 17 +++++++-- testing/av_test.c | 1 - toxav/bwcontroller.c | 6 ++-- toxav/rtp.c | 18 ++++++---- toxcore/network.c | 73 ++++++++++++++++++++++++++++++++------- toxcore/network.h | 28 +++++++++++++++ 6 files changed, 119 insertions(+), 24 deletions(-) diff --git a/auto_tests/network_test.c b/auto_tests/network_test.c index 15757d4d..6f535ee1 100644 --- a/auto_tests/network_test.c +++ b/auto_tests/network_test.c @@ -38,7 +38,10 @@ START_TEST(test_addr_resolv_localhost) int res = addr_resolve(localhost, &ip, nullptr); - ck_assert_msg(res > 0, "Resolver failed: %u, %s", errno, strerror(errno)); + int error = net_error(); + const char *strerror = net_new_strerror(error); + ck_assert_msg(res > 0, "Resolver failed: %d, %s", error, strerror); + net_kill_strerror(strerror); char ip_str[IP_NTOA_LEN]; ck_assert_msg(ip.family == TOX_AF_INET, "Expected family TOX_AF_INET, got %u.", ip.family); @@ -54,7 +57,10 @@ START_TEST(test_addr_resolv_localhost) localhost_split = 1; } - ck_assert_msg(res > 0, "Resolver failed: %u, %s", errno, strerror(errno)); + error = net_error(); + strerror = net_new_strerror(error); + ck_assert_msg(res > 0, "Resolver failed: %d, %s", error, strerror); + net_kill_strerror(strerror); ck_assert_msg(ip.family == TOX_AF_INET6, "Expected family TOX_AF_INET6 (%u), got %u.", TOX_AF_INET6, ip.family); IP6 ip6_loopback = get_ip6_loopback(); @@ -71,7 +77,10 @@ START_TEST(test_addr_resolv_localhost) IP extra; ip_reset(&extra); res = addr_resolve(localhost, &ip, &extra); - ck_assert_msg(res > 0, "Resolver failed: %u, %s", errno, strerror(errno)); + error = net_error(); + strerror = net_new_strerror(error); + ck_assert_msg(res > 0, "Resolver failed: %d, %s", error, strerror); + net_kill_strerror(strerror); #if USE_IPV6 ck_assert_msg(ip.family == TOX_AF_INET6, "Expected family TOX_AF_INET6 (%u), got %u.", TOX_AF_INET6, ip.family); @@ -157,6 +166,8 @@ static Suite *network_suite(void) { Suite *s = suite_create("Network"); + networking_at_startup(); + DEFTESTCASE(addr_resolv_localhost); DEFTESTCASE(ip_equal); diff --git a/testing/av_test.c b/testing/av_test.c index 0d61edec..5cc3b5dd 100644 --- a/testing/av_test.c +++ b/testing/av_test.c @@ -64,7 +64,6 @@ extern "C" { #include #include -#include #include #include #include diff --git a/toxav/bwcontroller.c b/toxav/bwcontroller.c index 5c6782ba..d786080e 100644 --- a/toxav/bwcontroller.c +++ b/toxav/bwcontroller.c @@ -145,8 +145,10 @@ void send_update(BWController *bwc) msg->recv = net_htonl(bwc->cycle.recv); if (-1 == m_send_custom_lossy_packet(bwc->m, bwc->friend_number, bwc_packet, sizeof(bwc_packet))) { - LOGGER_WARNING(bwc->m->log, "BWC send failed (len: %u)! std error: %s", - (unsigned)sizeof(bwc_packet), strerror(errno)); + const char *netstrerror = net_new_strerror(net_error()); + LOGGER_WARNING(bwc->m->log, "BWC send failed (len: %u)! std error: %s, net error %s", + (unsigned)sizeof(bwc_packet), strerror(errno), netstrerror); + net_kill_strerror(netstrerror); } } diff --git a/toxav/rtp.c b/toxav/rtp.c index c760d333..a65dbafe 100644 --- a/toxav/rtp.c +++ b/toxav/rtp.c @@ -802,8 +802,10 @@ int rtp_send_data(RTPSession *session, const uint8_t *data, uint32_t length, memcpy(rdata + 1 + RTP_HEADER_SIZE, data, length); if (-1 == m_send_custom_lossy_packet(session->m, session->friend_number, rdata, SIZEOF_VLA(rdata))) { - LOGGER_WARNING(session->m->log, "RTP send failed (len: %u)! std error: %s", - (unsigned)SIZEOF_VLA(rdata), strerror(errno)); + const char *netstrerror = net_new_strerror(net_error()); + LOGGER_WARNING(session->m->log, "RTP send failed (len: %u)! std error: %s, net error: %s", + (unsigned)SIZEOF_VLA(rdata), strerror(errno), netstrerror); + net_kill_strerror(netstrerror); } } else { /** @@ -819,8 +821,10 @@ int rtp_send_data(RTPSession *session, const uint8_t *data, uint32_t length, if (-1 == m_send_custom_lossy_packet(session->m, session->friend_number, rdata, piece + RTP_HEADER_SIZE + 1)) { - LOGGER_WARNING(session->m->log, "RTP send failed (len: %d)! std error: %s", - piece + RTP_HEADER_SIZE + 1, strerror(errno)); + const char *netstrerror = net_new_strerror(net_error()); + LOGGER_WARNING(session->m->log, "RTP send failed (len: %d)! std error: %s, net error: %s", + piece + RTP_HEADER_SIZE + 1, strerror(errno), netstrerror); + net_kill_strerror(netstrerror); } sent += piece; @@ -837,8 +841,10 @@ int rtp_send_data(RTPSession *session, const uint8_t *data, uint32_t length, if (-1 == m_send_custom_lossy_packet(session->m, session->friend_number, rdata, piece + RTP_HEADER_SIZE + 1)) { - LOGGER_WARNING(session->m->log, "RTP send failed (len: %d)! std error: %s", - piece + RTP_HEADER_SIZE + 1, strerror(errno)); + const char *netstrerror = net_new_strerror(net_error()); + LOGGER_WARNING(session->m->log, "RTP send failed (len: %d)! std error: %s, net error: %s", + piece + RTP_HEADER_SIZE + 1, strerror(errno), netstrerror); + net_kill_strerror(netstrerror); } } } diff --git a/toxcore/network.c b/toxcore/network.c index d88ba816..984071d4 100644 --- a/toxcore/network.c +++ b/toxcore/network.c @@ -63,15 +63,15 @@ #include #include +#define TOX_EWOULDBLOCK EWOULDBLOCK + #else #ifndef IPV6_V6ONLY #define IPV6_V6ONLY 27 #endif -#ifndef EWOULDBLOCK -#define EWOULDBLOCK WSAEWOULDBLOCK -#endif +#define TOX_EWOULDBLOCK WSAEWOULDBLOCK static const char *inet_ntop(Family family, const void *addr, char *buf, size_t bufsize) { @@ -376,10 +376,13 @@ static void loglogdata(Logger *log, const char *message, const uint8_t *buffer, char ip_str[IP_NTOA_LEN]; if (res < 0) { /* Windows doesn't necessarily know %zu */ + int error = net_error(); + const char *strerror = net_new_strerror(error); LOGGER_TRACE(log, "[%2u] %s %3u%c %s:%u (%u: %s) | %04x%04x", buffer[0], message, (buflen < 999 ? buflen : 999), 'E', - ip_ntoa(&ip_port.ip, ip_str, sizeof(ip_str)), net_ntohs(ip_port.port), errno, - strerror(errno), data_0(buflen, buffer), data_1(buflen, buffer)); + ip_ntoa(&ip_port.ip, ip_str, sizeof(ip_str)), net_ntohs(ip_port.port), error, + strerror, data_0(buflen, buffer), data_1(buflen, buffer)); + net_kill_strerror(strerror); } else if ((res > 0) && ((size_t)res <= buflen)) { LOGGER_TRACE(log, "[%2u] %s %3u%c %s:%u (%u: %s) | %04x%04x", buffer[0], message, (res < 999 ? res : 999), ((size_t)res < buflen ? '<' : '='), @@ -498,9 +501,12 @@ static int receivepacket(Logger *log, Socket sock, IP_Port *ip_port, uint8_t *da int fail_or_len = recvfrom(sock, (char *) data, MAX_UDP_PACKET_SIZE, 0, (struct sockaddr *)&addr, &addrlen); if (fail_or_len < 0) { + int error = net_error(); - if (fail_or_len < 0 && errno != EWOULDBLOCK) { - LOGGER_ERROR(log, "Unexpected error reading from socket: %u, %s", errno, strerror(errno)); + if (fail_or_len < 0 && error != TOX_EWOULDBLOCK) { + const char *strerror = net_new_strerror(error); + LOGGER_ERROR(log, "Unexpected error reading from socket: %u, %s", error, strerror); + net_kill_strerror(strerror); } return -1; /* Nothing received. */ @@ -681,7 +687,10 @@ Networking_Core *new_networking_ex(Logger *log, IP ip, uint16_t port_from, uint1 /* Check for socket error. */ if (!sock_valid(temp->sock)) { - LOGGER_ERROR(log, "Failed to get a socket?! %u, %s", errno, strerror(errno)); + int neterror = net_error(); + const char *strerror = net_new_strerror(neterror); + LOGGER_ERROR(log, "Failed to get a socket?! %d, %s", neterror, strerror); + net_kill_strerror(strerror); free(temp); if (error) { @@ -769,8 +778,11 @@ Networking_Core *new_networking_ex(Logger *log, IP ip, uint16_t port_from, uint1 mreq.ipv6mr_interface = 0; int res = setsockopt(temp->sock, IPPROTO_IPV6, IPV6_ADD_MEMBERSHIP, (const char *)&mreq, sizeof(mreq)); - LOGGER_DEBUG(log, res < 0 ? "Failed to activate local multicast membership. (%u, %s)" : - "Local multicast group FF02::1 joined successfully", errno, strerror(errno)); + int neterror = net_error(); + const char *strerror = net_new_strerror(neterror); + LOGGER_DEBUG(log, res < 0 ? "Failed to activate local multicast membership. (%d, %s)" : + "Local multicast group FF02::1 joined successfully", neterror, strerror); + net_kill_strerror(strerror); } /* a hanging program or a different user might block the standard port; @@ -827,9 +839,11 @@ Networking_Core *new_networking_ex(Logger *log, IP ip, uint16_t port_from, uint1 } char ip_str[IP_NTOA_LEN]; - LOGGER_ERROR(log, "Failed to bind socket: %u, %s IP: %s port_from: %u port_to: %u", errno, strerror(errno), + int neterror = net_error(); + const 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_kill_strerror(strerror); kill_networking(temp); if (error) { @@ -1520,3 +1534,38 @@ size_t net_unpack_u64(const uint8_t *bytes, uint64_t *v) *v = ((uint64_t)hi << 32) | lo; return p - bytes; } + +int net_error(void) +{ +#if defined(_WIN32) || defined(__WIN32__) || defined(WIN32) + return WSAGetLastError(); +#else + return errno; +#endif +} + +const char *net_new_strerror(int error) +{ +#if defined(_WIN32) || defined(__WIN32__) || defined(WIN32) + char *str = nullptr; + // Windows API is weird. The 5th function arg is of char* type, but we + // have to pass char** so that it could assign new memory block to our + // pointer, so we have to cast our char** to char* for the compilation + // not to fail (otherwise it would fail to find a variant of this function + // accepting char** as the 5th arg) and Windows inside casts it back + // to char** to do the assignment. So no, this cast you see here, although + // it looks weird, is not a mistake. + FormatMessageA(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, nullptr, + error, 0, (char *)&str, 0, nullptr); + return str; +#else + return strerror(error); +#endif +} + +void net_kill_strerror(const char *strerror) +{ +#if defined(_WIN32) || defined(__WIN32__) || defined(WIN32) + LocalFree((char *)strerror); +#endif +} diff --git a/toxcore/network.h b/toxcore/network.h index 405721b2..10ddef02 100644 --- a/toxcore/network.h +++ b/toxcore/network.h @@ -404,6 +404,34 @@ void net_freeipport(IP_Port *ip_ports); */ int bind_to_port(Socket sock, int family, uint16_t port); +/* Get the last networking error code. + * + * Similar to Unix's errno, but cross-platform, as not all platforms use errno + * to indicate networking errors. + * + * Note that different platforms may return different codes for the same error, + * so you likely shouldn't be checking the value returned by this function + * unless you know what you are doing, you likely just want to use it in + * combination with net_new_strerror() to print the error. + * + * return platform-dependent network error code, if any. + */ +int net_error(void); + +/* Get a text explanation for the error code from net_error(). + * + * return NULL on failure. + * return pointer to a NULL-terminated string describing the error code on + * success. The returned string must be freed using net_kill_strerror(). + */ +const char *net_new_strerror(int error); + +/* Frees the string returned by net_new_strerror(). + * It's valid to pass NULL as the argument, the function does nothing in this + * case. + */ +void net_kill_strerror(const char *strerror); + /* Initialize networking. * bind to ip and port. * ip must be in network order EX: 127.0.0.1 = (7F000001).