From 1a3ea0e76e2d574abbde0c3741ac88e6fbcbee53 Mon Sep 17 00:00:00 2001 From: iphydf Date: Fri, 10 Dec 2021 21:58:57 +0000 Subject: [PATCH] test: Make ERROR logging fatal in tests. This doesn't currently work, because we get a lot of errors during tests. This should not happen. Either those errors are warnings, or something is wrong with either the code or the test. --- testing/misc_tools.c | 5 +++++ toxcore/Messenger.c | 12 ++++++------ toxcore/ccompat.h | 2 ++ toxcore/group.c | 4 ++-- toxcore/logger.c | 2 +- toxcore/network.c | 19 ++++++++++++++----- toxcore/tox.c | 15 +++++++-------- 7 files changed, 37 insertions(+), 22 deletions(-) diff --git a/testing/misc_tools.c b/testing/misc_tools.c index 8ab23db93e..7fb6e322b2 100644 --- a/testing/misc_tools.c +++ b/testing/misc_tools.c @@ -171,6 +171,11 @@ void print_debug_log(Tox *m, Tox_Log_Level level, const char *file, uint32_t lin uint32_t index = user_data ? *(uint32_t *)user_data : 0; fprintf(stderr, "[#%u] %s %s:%u\t%s:\t%s\n", index, tox_log_level_name(level), file, line, func, message); + + if (level == TOX_LOG_LEVEL_ERROR) { + fputs("Aborting test program\n", stderr); + abort(); + } } Tox *tox_new_log_lan(struct Tox_Options *options, Tox_Err_New *err, void *log_user_data, bool lan_discovery) diff --git a/toxcore/Messenger.c b/toxcore/Messenger.c index 7e13fe2cf7..5ad1b5be36 100644 --- a/toxcore/Messenger.c +++ b/toxcore/Messenger.c @@ -494,22 +494,22 @@ int m_send_message_generic(Messenger *m, int32_t friendnumber, uint8_t type, con uint32_t *message_id) { if (type > MESSAGE_ACTION) { - LOGGER_ERROR(m->log, "Message type %d is invalid", type); + LOGGER_WARNING(m->log, "Message type %d is invalid", type); return -5; } if (!friend_is_valid(m, friendnumber)) { - LOGGER_ERROR(m->log, "Friend number %d is invalid", friendnumber); + LOGGER_WARNING(m->log, "Friend number %d is invalid", friendnumber); return -1; } if (length >= MAX_CRYPTO_DATA_SIZE) { - LOGGER_ERROR(m->log, "Message length %u is too large", length); + LOGGER_WARNING(m->log, "Message length %u is too large", length); return -2; } if (m->friendlist[friendnumber].status != FRIEND_ONLINE) { - LOGGER_ERROR(m->log, "Friend %d is not online", friendnumber); + LOGGER_WARNING(m->log, "Friend %d is not online", friendnumber); return -3; } @@ -524,8 +524,8 @@ int m_send_message_generic(Messenger *m, int32_t friendnumber, uint8_t type, con m->friendlist[friendnumber].friendcon_id), packet, length + 1, 0); if (packet_num == -1) { - LOGGER_ERROR(m->log, "Failed to write crypto packet for message of length %d to friend %d", - length, friendnumber); + LOGGER_WARNING(m->log, "Failed to write crypto packet for message of length %d to friend %d", + length, friendnumber); return -4; } diff --git a/toxcore/ccompat.h b/toxcore/ccompat.h index 6f870de985..662644f943 100644 --- a/toxcore/ccompat.h +++ b/toxcore/ccompat.h @@ -4,6 +4,8 @@ #ifndef C_TOXCORE_TOXCORE_CCOMPAT_H #define C_TOXCORE_TOXCORE_CCOMPAT_H +#include + //!TOKSTYLE- // Variable length arrays. diff --git a/toxcore/group.c b/toxcore/group.c index d2cd577042..6cad97f048 100644 --- a/toxcore/group.c +++ b/toxcore/group.c @@ -752,8 +752,8 @@ static int cmp_u64(uint64_t a, uint64_t b) /* Order peers with friends first and with more recently active earlier */ static int cmp_frozen(const void *a, const void *b) { - const Group_Peer *pa = (const Group_Peer *) a; - const Group_Peer *pb = (const Group_Peer *) b; + const Group_Peer *pa = (const Group_Peer *)a; + const Group_Peer *pb = (const Group_Peer *)b; if (pa->is_friend ^ pb->is_friend) { return pa->is_friend ? -1 : 1; diff --git a/toxcore/logger.c b/toxcore/logger.c index 67feeeb3ba..ba9234cb82 100644 --- a/toxcore/logger.c +++ b/toxcore/logger.c @@ -98,7 +98,7 @@ void logger_write(const Logger *log, Logger_Level level, const char *file, int l // The full path may contain PII of the person compiling toxcore (their // username and directory layout). const char *filename = strrchr(file, '/'); - file = filename ? filename + 1 : file; + file = filename != nullptr ? filename + 1 : file; #if defined(_WIN32) || defined(__CYGWIN__) // On Windows, the path separator *may* be a backslash, so we look for that // one too. diff --git a/toxcore/network.c b/toxcore/network.c index 0eaad16e7b..bedcabc6a7 100644 --- a/toxcore/network.c +++ b/toxcore/network.c @@ -500,14 +500,18 @@ uint16_t net_port(const Networking_Core *net) int sendpacket(Networking_Core *net, IP_Port ip_port, const uint8_t *data, uint16_t length) { if (net_family_is_unspec(net->family)) { /* Socket not initialized */ - LOGGER_ERROR(net->log, "attempted to send message of length %u on uninitialised socket", (unsigned)length); + // TODO(iphydf): Make this an error. Currently, the onion client calls + // this via DHT getnodes. + LOGGER_WARNING(net->log, "attempted to send message of length %u on uninitialised socket", (unsigned)length); return -1; } /* socket TOX_AF_INET, but target IP NOT: can't send */ if (net_family_is_ipv4(net->family) && !net_family_is_ipv4(ip_port.ip.family)) { - LOGGER_ERROR(net->log, "attempted to send message with network family %d (probably IPv6) on IPv4 socket", - ip_port.ip.family.value); + // TODO(iphydf): Make this an error. Occasionally we try to send to an + // all-zero ip_port. + LOGGER_WARNING(net->log, "attempted to send message with network family %d (probably IPv6) on IPv4 socket", + ip_port.ip.family.value); return -1; } @@ -548,6 +552,8 @@ int sendpacket(Networking_Core *net, IP_Port ip_port, const uint8_t *data, uint1 addr6->sin6_flowinfo = 0; addr6->sin6_scope_id = 0; } else { + // TODO(iphydf): Make this an error. Currently this fails sometimes when + // called from DHT.c:do_ping_and_sendnode_requests. LOGGER_WARNING(net->log, "unknown address type: %d", ip_port.ip.family.value); return -1; } @@ -651,12 +657,15 @@ void networking_poll(Networking_Core *net, void *userdata) continue; } - if (!(net->packethandlers[data[0]].function)) { + packet_handler_cb *const cb = net->packethandlers[data[0]].function; + void *const object = net->packethandlers[data[0]].object; + + if (cb == nullptr) { LOGGER_WARNING(net->log, "[%02u] -- Packet has no handler", data[0]); continue; } - net->packethandlers[data[0]].function(net->packethandlers[data[0]].object, ip_port, data, length, userdata); + cb(object, ip_port, data, length, userdata); } } diff --git a/toxcore/tox.c b/toxcore/tox.c index e7bc5b1ce5..459e8272a9 100644 --- a/toxcore/tox.c +++ b/toxcore/tox.c @@ -702,15 +702,16 @@ bool tox_bootstrap(Tox *tox, const char *host, uint16_t port, const uint8_t *pub return 0; } - unsigned int i; - lock(tox); - for (i = 0; i < count; ++i) { + for (int32_t i = 0; i < count; ++i) { root[i].port = net_htons(port); onion_add_bs_path_node(tox->m->onion_c, root[i], public_key); - dht_bootstrap(tox->m->dht, root[i], public_key); + + if (!tox->m->options.udp_disabled) { + dht_bootstrap(tox->m->dht, root[i], public_key); + } } unlock(tox); @@ -743,7 +744,7 @@ bool tox_add_tcp_relay(Tox *tox, const char *host, uint16_t port, const uint8_t IP_Port *root; - int32_t count = net_getipport(host, &root, TOX_SOCK_STREAM); + const int32_t count = net_getipport(host, &root, TOX_SOCK_STREAM); if (count == -1) { net_freeipport(root); @@ -751,11 +752,9 @@ bool tox_add_tcp_relay(Tox *tox, const char *host, uint16_t port, const uint8_t return 0; } - unsigned int i; - lock(tox); - for (i = 0; i < count; ++i) { + for (int32_t i = 0; i < count; ++i) { root[i].port = net_htons(port); add_tcp_relay(tox->m->net_crypto, root[i], public_key);