From 71758f20af4014f968f988c147f5e890502937d0 Mon Sep 17 00:00:00 2001 From: iphydf Date: Fri, 14 Jan 2022 23:54:06 +0000 Subject: [PATCH] cleanup: Stop using `strerror` directly. We have a more portable wrapper that is now also thread-safe. Also stopped using sprintf in the one place we used it. This doesn't really help much, but it allows us to forbid sprintf globally. --- auto_tests/network_test.c | 2 +- .../docker/tox-bootstrapd.sha256 | 2 +- toxav/bwcontroller.c | 6 ++-- toxav/rtp.c | 18 +++++------ toxav/toxav.c | 4 ++- toxcore/Messenger.c | 2 +- toxcore/TCP_client.c | 2 +- toxcore/network.c | 30 ++++++++++++++----- toxcore/network.h | 4 +-- 9 files changed, 44 insertions(+), 26 deletions(-) diff --git a/auto_tests/network_test.c b/auto_tests/network_test.c index 29425337e0..d8df842228 100644 --- a/auto_tests/network_test.c +++ b/auto_tests/network_test.c @@ -28,7 +28,7 @@ START_TEST(test_addr_resolv_localhost) int res = addr_resolve(localhost, &ip, nullptr); int error = net_error(); - const char *strerror = net_new_strerror(error); + char *strerror = net_new_strerror(error); ck_assert_msg(res > 0, "Resolver failed: %d, %s", error, strerror); net_kill_strerror(strerror); diff --git a/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 b/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 index 8628c8d970..7b4b6e782c 100644 --- a/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 +++ b/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 @@ -1 +1 @@ -7f4c96481e30156e3c2e7e448e70faaaa12911694390374ae09bd53d5d7b4f9c /usr/local/bin/tox-bootstrapd +fc6b060abddd1898d97b0cf067d19c7a9d68999469690d91e7cf59327e700dbf /usr/local/bin/tox-bootstrapd diff --git a/toxav/bwcontroller.c b/toxav/bwcontroller.c index cdc1cdc8cc..eabc56a760 100644 --- a/toxav/bwcontroller.c +++ b/toxav/bwcontroller.c @@ -148,9 +148,11 @@ static void send_update(BWController *bwc) assert(offset == sizeof(bwc_packet)); if (bwc_send_custom_lossy_packet(bwc->tox, bwc->friend_number, bwc_packet, sizeof(bwc_packet)) == -1) { - const char *netstrerror = net_new_strerror(net_error()); + char *netstrerror = net_new_strerror(net_error()); + char *stdstrerror = net_new_strerror(errno); LOGGER_WARNING(bwc->m->log, "BWC send failed (len: %u)! std error: %s, net error %s", - (unsigned)sizeof(bwc_packet), strerror(errno), netstrerror); + (unsigned)sizeof(bwc_packet), stdstrerror, netstrerror); + net_kill_strerror(stdstrerror); net_kill_strerror(netstrerror); } } diff --git a/toxav/rtp.c b/toxav/rtp.c index 731d86d818..8b24503242 100644 --- a/toxav/rtp.c +++ b/toxav/rtp.c @@ -813,9 +813,9 @@ int rtp_send_data(RTPSession *session, const uint8_t *data, uint32_t length, memcpy(rdata + 1 + RTP_HEADER_SIZE, data, length); if (-1 == rtp_send_custom_lossy_packet(session->tox, session->friend_number, rdata, SIZEOF_VLA(rdata))) { - 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); + char *netstrerror = net_new_strerror(net_error()); + LOGGER_WARNING(session->m->log, "RTP send failed (len: %u)! net error: %s", + (unsigned)SIZEOF_VLA(rdata), netstrerror); net_kill_strerror(netstrerror); } } else { @@ -832,9 +832,9 @@ int rtp_send_data(RTPSession *session, const uint8_t *data, uint32_t length, if (-1 == rtp_send_custom_lossy_packet(session->tox, session->friend_number, rdata, piece + RTP_HEADER_SIZE + 1)) { - 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); + char *netstrerror = net_new_strerror(net_error()); + LOGGER_WARNING(session->m->log, "RTP send failed (len: %d)! net error: %s", + piece + RTP_HEADER_SIZE + 1, netstrerror); net_kill_strerror(netstrerror); } @@ -852,9 +852,9 @@ int rtp_send_data(RTPSession *session, const uint8_t *data, uint32_t length, if (-1 == rtp_send_custom_lossy_packet(session->tox, session->friend_number, rdata, piece + RTP_HEADER_SIZE + 1)) { - 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); + char *netstrerror = net_new_strerror(net_error()); + LOGGER_WARNING(session->m->log, "RTP send failed (len: %d)! net error: %s", + piece + RTP_HEADER_SIZE + 1, netstrerror); net_kill_strerror(netstrerror); } } diff --git a/toxav/toxav.c b/toxav/toxav.c index 5d726674f5..2c409e8ca5 100644 --- a/toxav/toxav.c +++ b/toxav/toxav.c @@ -911,7 +911,9 @@ static Toxav_Err_Send_Frame send_frames(const Logger *log, ToxAVCall *call) LOGGER_DEBUG(log, "+ _sending_FRAME_ b0=%d b1=%d", buf[0], buf[1]); if (res < 0) { - LOGGER_WARNING(log, "Could not send video frame: %s", strerror(errno)); + char *netstrerror = net_new_strerror(net_error()); + LOGGER_WARNING(log, "Could not send video frame: %s", netstrerror); + net_kill_strerror(netstrerror); return TOXAV_ERR_SEND_FRAME_RTP_FAILED; } } diff --git a/toxcore/Messenger.c b/toxcore/Messenger.c index 94e2c47fb3..4ccd106ecf 100644 --- a/toxcore/Messenger.c +++ b/toxcore/Messenger.c @@ -2475,7 +2475,7 @@ static char *id_to_string(const uint8_t *pk, char *id_str, size_t length) } for (uint32_t i = 0; i < CRYPTO_PUBLIC_KEY_SIZE; ++i) { - sprintf(&id_str[i * 2], "%02X", pk[i]); + snprintf(&id_str[i * 2], length - i * 2, "%02X", pk[i]); } id_str[CRYPTO_PUBLIC_KEY_SIZE * 2] = 0; diff --git a/toxcore/TCP_client.c b/toxcore/TCP_client.c index 6fe2337b63..04584f9d6e 100644 --- a/toxcore/TCP_client.c +++ b/toxcore/TCP_client.c @@ -161,7 +161,7 @@ static int proxy_http_read_connection_response(const Logger *logger, TCP_Client_ data[sizeof(data) - 1] = 0; - if (strstr((char *)data, success)) { + if (strstr((const char *)data, success)) { // drain all data unsigned int data_left = net_socket_data_recv_buffer(tcp_conn->sock); diff --git a/toxcore/network.c b/toxcore/network.c index 81a9ad333f..e727dbd5af 100644 --- a/toxcore/network.c +++ b/toxcore/network.c @@ -491,7 +491,7 @@ static void loglogdata(const Logger *log, const char *message, const uint8_t *bu if (res < 0) { /* Windows doesn't necessarily know `%zu` */ int error = net_error(); - const char *strerror = net_new_strerror(error); + char *strerror = net_new_strerror(error); LOGGER_TRACE(log, "[%2u] %s %3u%c %s:%u (%u: %s) | %04x%04x", buffer[0], message, min_u16(buflen, 999), 'E', ip_ntoa(&ip_port.ip, ip_str, sizeof(ip_str)), net_ntohs(ip_port.port), error, @@ -639,7 +639,7 @@ static int receivepacket(const Logger *log, Socket sock, IP_Port *ip_port, uint8 int error = net_error(); if (!should_ignore_recv_error(error)) { - const char *strerror = net_new_strerror(error); + char *strerror = net_new_strerror(error); LOGGER_ERROR(log, "Unexpected error reading from socket: %u, %s", error, strerror); net_kill_strerror(strerror); } @@ -836,7 +836,7 @@ Networking_Core *new_networking_ex(const Logger *log, IP ip, uint16_t port_from, /* Check for socket error. */ if (!sock_valid(temp->sock)) { int neterror = net_error(); - const char *strerror = net_new_strerror(neterror); + char *strerror = net_new_strerror(neterror); LOGGER_ERROR(log, "Failed to get a socket?! %d, %s", neterror, strerror); net_kill_strerror(strerror); free(temp); @@ -945,7 +945,7 @@ Networking_Core *new_networking_ex(const Logger *log, IP ip, uint16_t port_from, const int res = setsockopt(temp->sock.socket, IPPROTO_IPV6, IPV6_ADD_MEMBERSHIP, (const char *)&mreq, sizeof(mreq)); int neterror = net_error(); - const char *strerror = net_new_strerror(neterror); + char *strerror = net_new_strerror(neterror); if (res < 0) { LOGGER_DEBUG(log, "Failed to activate local multicast membership. (%d, %s)", neterror, strerror); @@ -1017,7 +1017,7 @@ Networking_Core *new_networking_ex(const Logger *log, IP ip, uint16_t port_from, char ip_str[IP_NTOA_LEN]; int neterror = net_error(); - const char *strerror = net_new_strerror(neterror); + 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); @@ -1703,7 +1703,7 @@ int net_error(void) #endif } -const char *net_new_strerror(int error) +char *net_new_strerror(int error) { #if defined(_WIN32) || defined(__WIN32__) || defined(WIN32) char *str = nullptr; @@ -1718,13 +1718,27 @@ const char *net_new_strerror(int error) error, 0, (char *)&str, 0, nullptr); return str; #else - return strerror(error); + char *str = (char *)malloc(256); +#ifdef _GNU_SOURCE + str = strerror_r(error, str, 256); +#else + const int fmt_error = strerror_r(error, str, 256); + + if (fmt_error != 0) { + snprintf(str, 256, "error %d (strerror failed with error %d)", error, fmt_error); + } + +#endif + + return str; #endif } -void net_kill_strerror(const char *strerror) +void net_kill_strerror(char *strerror) { #if defined(_WIN32) || defined(__WIN32__) || defined(WIN32) LocalFree((char *)strerror); +#else + free(strerror); #endif } diff --git a/toxcore/network.h b/toxcore/network.h index efc1988f6d..5aa4a916a0 100644 --- a/toxcore/network.h +++ b/toxcore/network.h @@ -430,13 +430,13 @@ int net_error(void); * 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); +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); +void net_kill_strerror(char *strerror); /** Initialize networking. * Added for reverse compatibility with old new_networking calls.