From 1331a32223da0090ab982eabd4e04f3d8e36b6af Mon Sep 17 00:00:00 2001 From: "Coren[m]" Date: Sat, 5 Oct 2013 12:53:54 +0200 Subject: [PATCH] Broken *_wait() into *_wait_prepare() and *_wait_execute() To allow the actual waiting to run without any locking, split it into preparing the data it uses and the execution of the wait. The caller must provide with the buffer to store whatever data it requires to wait. Completely eliminates any reliance on the existence of anything but that data in the actual wait routine. Also fixed a few argument type warnings inside LOGGING. --- auto_tests/messenger_test.c | 4 +- toxcore/Messenger.c | 18 ++++---- toxcore/Messenger.h | 12 ++---- toxcore/network.c | 86 +++++++++++++++++++++++-------------- toxcore/network.h | 12 ++---- toxcore/tox.c | 19 ++++---- toxcore/tox.h | 18 ++++++-- 7 files changed, 94 insertions(+), 75 deletions(-) diff --git a/auto_tests/messenger_test.c b/auto_tests/messenger_test.c index 6ede62ef..605fa4f3 100644 --- a/auto_tests/messenger_test.c +++ b/auto_tests/messenger_test.c @@ -253,7 +253,7 @@ START_TEST(test_dht_state_saveloadsave) char msg[128]; size_t offset = res >> 4; uint8_t *ptr = buffer + extra + offset; - sprintf(msg, "Failed to load back stored buffer: 0x%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx @%u/%u, code %d", + sprintf(msg, "Failed to load back stored buffer: 0x%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx @%zu/%zu, code %d", ptr[-2], ptr[-1], ptr[0], ptr[1], ptr[2], ptr[3], ptr[4], ptr[5], offset, size, res & 0x0F); ck_assert_msg(res == 0, msg); } @@ -295,7 +295,7 @@ START_TEST(test_messenger_state_saveloadsave) char msg[128]; size_t offset = res >> 4; uint8_t *ptr = buffer + extra + offset; - sprintf(msg, "Failed to load back stored buffer: 0x%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx @%u/%u, code %d", + sprintf(msg, "Failed to load back stored buffer: 0x%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx @%zu/%zu, code %d", ptr[-2], ptr[-1], ptr[0], ptr[1], ptr[2], ptr[3], ptr[4], ptr[5], offset, size, res & 0x0F); ck_assert_msg(res == 0, msg); } diff --git a/toxcore/Messenger.c b/toxcore/Messenger.c index 09095e5f..e4387fcb 100644 --- a/toxcore/Messenger.c +++ b/toxcore/Messenger.c @@ -1390,18 +1390,16 @@ void doMessenger(Messenger *m) } /* - * Waits for something to happen on the socket for up to milliseconds milliseconds - * *** Function MUSTN'T poll. *** - * The function mustn't modify anything at all, so it can be called completely - * asynchronously without any worry. - * - * returns 0 if the timeout was reached - * returns 1 if there is socket activity (i.e. tox_do() should be called) - * + * functions to avoid excessive polling */ -int waitMessenger(Messenger *m, uint16_t milliseconds) +int waitprepareMessenger(Messenger *m, uint8_t *data, uint16_t *lenptr) { - return networking_wait(m->net, sendqueue_total(m->net_crypto->lossless_udp), milliseconds); + return networking_wait_prepare(m->net, sendqueue_total(m->net_crypto->lossless_udp), data, lenptr); +} + +int waitexecuteMessenger(Messenger *m, uint8_t *data, uint16_t len, uint16_t milliseconds) +{ + return networking_wait_execute(data, len, milliseconds); }; /* return size of the messenger data (for saving) */ diff --git a/toxcore/Messenger.h b/toxcore/Messenger.h index 27e2c502..80450b1a 100644 --- a/toxcore/Messenger.h +++ b/toxcore/Messenger.h @@ -456,16 +456,10 @@ void cleanupMessenger(Messenger *M); void doMessenger(Messenger *m); /* - * Waits for something to happen on the socket for up to milliseconds milliseconds - * *** Function MUSTN'T poll. *** - * The function mustn't modify anything at all, so it can be called completely - * asynchronously without any worry. - * - * returns 0 if the timeout was reached - * returns 1 if there is socket activity (i.e. tox_do() should be called) - * + * functions to avoid excessive polling */ -int waitMessenger(Messenger *m, uint16_t milliseconds); +int waitprepareMessenger(Messenger *m, uint8_t *data, uint16_t *lenptr); +int waitexecuteMessenger(Messenger *m, uint8_t *data, uint16_t len, uint16_t milliseconds); /* SAVING AND LOADING FUNCTIONS: */ diff --git a/toxcore/network.c b/toxcore/network.c index a5786801..3e533799 100644 --- a/toxcore/network.c +++ b/toxcore/network.c @@ -25,6 +25,10 @@ #include "config.h" #endif +#ifndef WIN32 +#include +#endif + #include "network.h" #include "util.h" @@ -300,36 +304,53 @@ void networking_poll(Networking_Core *net) } /* - * Waits for something to happen on the socket for up to milliseconds milliseconds - * *** Function MUSTN'T poll. *** - * The function mustn't modify anything at all, so it can be called completely - * asynchronously without any worry. - * - * returns 0 if the timeout was reached - * returns 1 if there is socket activity (i.e. tox_do() should be called) - * + * function to avoid excessive polling */ -int networking_wait(Networking_Core *net, uint32_t sendqueue_len, uint16_t milliseconds) +typedef struct +{ + sock_t sock; + uint32_t sendqueue_length; +} select_info; + +int networking_wait_prepare(Networking_Core *net, uint32_t sendqueue_length, uint8_t *data, uint16_t *lenptr) +{ + if ((data == NULL) || (*lenptr < sizeof(select_info))) + { + *lenptr = sizeof(select_info); + return 0; + } + + *lenptr = sizeof(select_info); + select_info *s = (select_info *)data; + s->sock = net->sock; + s->sendqueue_length = sendqueue_length; + + return 1; +} + +int networking_wait_execute(uint8_t *data, uint16_t len, uint16_t milliseconds) { - sock_t sock = net->sock; /* WIN32: supported since Win2K, but might need some adjustements */ /* UNIX: this should work for any remotely Unix'ish system */ - int nfds = 1 + sock; + + select_info *s = (select_info *)data; + + int nfds = 1 + s->sock; /* the FD_ZERO calls might be superfluous */ fd_set readfds; FD_ZERO(&readfds); - FD_SET(sock, &readfds); + FD_SET(s->sock, &readfds); fd_set writefds; FD_ZERO(&writefds); /* add only if we have packets queued, signals that a write won't block */ - if (sendqueue_len > 0) - FD_SET(sock, &writefds); + if (s->sendqueue_length > 0) + FD_SET(s->sock, &writefds); fd_set exceptfds; FD_ZERO(&exceptfds); - FD_SET(sock, &exceptfds); + FD_SET(s->sock, &exceptfds); struct timeval timeout; timeout.tv_sec = 0; @@ -342,8 +363,8 @@ int networking_wait(Networking_Core *net, uint32_t sendqueue_len, uint16_t milli int res = select(nfds, &readfds, &writefds, &exceptfds, &timeout); #ifdef LOGGING sprintf(logbuffer, "select(%d): %d (%d, %s) - %d %d %d\n", milliseconds, res, errno, - strerror(errno), FD_ISSET(sock, &readfds), FD_ISSET(sock, &writefds), - FD_ISSET(sock, &exceptfds)); + strerror(errno), FD_ISSET(s->sock, &readfds), FD_ISSET(s->sock, &writefds), + FD_ISSET(s->sock, &exceptfds)); loglog(logbuffer); #endif @@ -972,26 +993,27 @@ int addr_resolve_or_parse_ip(const char *address, IP *to, IP *extra) }; #ifdef LOGGING +static char errmsg_ok[3] = "OK"; static void loglogdata(char *message, uint8_t *buffer, size_t buflen, IP_Port *ip_port, ssize_t res) { + uint16_t port = ntohs(ip_port->port); + uint32_t data[2]; + data[0] = buflen > 4 ? ntohl(*(uint32_t *)&buffer[1]) : 0; + data[1] = buflen > 7 ? ntohl(*(uint32_t *)&buffer[5]) : 0; if (res < 0) - snprintf(logbuffer, sizeof(logbuffer), "[%2u] %s %3u%c %s:%u (%d: %s) | %04x%04x\n", - buffer[0], message, buflen < 999 ? buflen : 999, 'E', - ip_ntoa(&ip_port->ip), ntohs(ip_port->port), errno, - strerror(errno), buflen > 4 ? ntohl(*(uint32_t *)&buffer[1]) : 0, - (buflen > 7) ? ntohl(*(uint32_t *)(&buffer[5])) : 0); + { + int written = snprintf(logbuffer, sizeof(logbuffer), "[%2u] %s %3hu%c %s:%hu (%u: %s) | %04x%04x\n", + buffer[0], message, (buflen < 999 ? (uint16_t)buflen : 999), 'E', + ip_ntoa(&ip_port->ip), port, errno, strerror(errno), data[0], data[1]); + } else if ((res > 0) && ((size_t)res <= buflen)) - snprintf(logbuffer, sizeof(logbuffer), "[%2u] %s %3u%c %s:%u (%d: %s) | %04x%04x\n", - buffer[0], message, res < 999 ? res : 999, (size_t)res < buflen ? '<' : '=', - ip_ntoa(&ip_port->ip), ntohs(ip_port->port), 0, - "OK", buflen > 4 ? ntohl(*(uint32_t *)&buffer[1]) : 0, - (buflen > 7) ? ntohl(*(uint32_t *)(&buffer[5])) : 0); + snprintf(logbuffer, sizeof(logbuffer), "[%2u] %s %3zu%c %s:%hu (%u: %s) | %04x%04x\n", + buffer[0], message, (res < 999 ? (size_t)res : 999), ((size_t)res < buflen ? '<' : '='), + ip_ntoa(&ip_port->ip), port, 0, errmsg_ok, data[0], data[1]); else /* empty or overwrite */ - snprintf(logbuffer, sizeof(logbuffer), "[%2u] %s %u%c%u %s:%u (%d: %s) | %04x%04x\n", - buffer[0], message, res, !res ? '!' : '>', buflen, - ip_ntoa(&ip_port->ip), ntohs(ip_port->port), 0, - "OK", buflen > 4 ? ntohl(*(uint32_t *)&buffer[1]) : 0, - (buflen > 7) ? ntohl(*(uint32_t *)(&buffer[5])) : 0); + snprintf(logbuffer, sizeof(logbuffer), "[%2u] %s %zu%c%zu %s:%hu (%u: %s) | %04x%04x\n", + buffer[0], message, (size_t)res, (!res ? '!' : '>'), buflen, + ip_ntoa(&ip_port->ip), port, 0, errmsg_ok, data[0], data[1]); logbuffer[sizeof(logbuffer) - 1] = 0; loglog(logbuffer); diff --git a/toxcore/network.h b/toxcore/network.h index d143b48d..54df7d2e 100644 --- a/toxcore/network.h +++ b/toxcore/network.h @@ -265,16 +265,10 @@ void networking_registerhandler(Networking_Core *net, uint8_t byte, packet_handl void networking_poll(Networking_Core *net); /* - * Waits for something to happen on the socket for up to milliseconds milliseconds - * *** Function MUSTN'T poll. *** - * The function mustn't modify anything at all, so it can be called completely - * asynchronously without any worry. - * - * returns 0 if the timeout was reached - * returns 1 if there is socket activity (i.e. tox_do() should be called) - * + * functions to avoid excessive polling */ -int networking_wait(Networking_Core *net, uint32_t sendqueue_len, uint16_t milliseconds); +int networking_wait_prepare(Networking_Core *net, uint32_t sendqueue_length, uint8_t *data, uint16_t *lenptr); +int networking_wait_execute(uint8_t *data, uint16_t len, uint16_t milliseconds); /* Initialize networking. * bind to ip and port. diff --git a/toxcore/tox.c b/toxcore/tox.c index 080909d3..b2107389 100644 --- a/toxcore/tox.c +++ b/toxcore/tox.c @@ -537,19 +537,18 @@ void tox_do(Tox *tox) } /* - * Waits for something to happen on the socket for up to milliseconds milliseconds - * *** Function MUSTN'T poll. *** - * The function mustn't modify anything at all, so it can be called completely - * asynchronously without any worry. - * - * returns 0 if the timeout was reached - * returns 1 if there is socket activity (i.e. tox_do() should be called) - * + * functions to avoid excessive polling */ -int tox_wait(Tox *tox, uint16_t milliseconds) +int tox_wait_prepare(Tox *tox, uint8_t *data, uint16_t *lenptr) { Messenger *m = tox; - waitMessenger(m, milliseconds); + waitprepareMessenger(m, data, lenptr); +} + +int tox_wait_execute(Tox *tox, uint8_t *data, uint16_t len, uint16_t milliseconds) +{ + Messenger *m = tox; + waitexecuteMessenger(m, data, len, milliseconds); } /* SAVING AND LOADING FUNCTIONS: */ diff --git a/toxcore/tox.h b/toxcore/tox.h index e497e6f5..3278a9d3 100644 --- a/toxcore/tox.h +++ b/toxcore/tox.h @@ -482,16 +482,28 @@ void tox_kill(Tox *tox); void tox_do(Tox *tox); /* + * tox_wait_prepare(): function should be called under lock + * Prepares the data required to call tox_wait_execute() asynchronously + * + * data[] is reserved and kept by the caller + * len is in/out: in = reserved data[], out = required data[] + * + * returns 1 on success + * returns 0 on failure (length is insufficient) + * + * tox_wait_execute(): function can be called asynchronously * Waits for something to happen on the socket for up to milliseconds milliseconds. * *** Function MUSTN'T poll. *** * The function mustn't modify anything at all, so it can be called completely * asynchronously without any worry. * - * returns 0 if the timeout was reached - * returns 1 if there is socket activity (i.e. tox_do() should be called) + * returns 1 if there is socket activity (i.e. tox_do() should be called) + * returns 0 if the timeout was reached + * returns -1 if data was NULL or len too short * */ -int tox_wait(Tox *tox, uint16_t milliseconds); +int tox_wait_prepare(Tox *tox, uint8_t *data, uint16_t *lenptr); +int tox_wait_execute(Tox *tox, uint8_t *data, uint16_t len, uint16_t milliseconds); /* SAVING AND LOADING FUNCTIONS: */