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

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

toxcore/Messenger.c

@@ -1390,18 +1390,16 @@ void doMessenger(Messenger *m)
 }
 
 /*
- * Waits for something to happen on the socket for up to milliseconds milliseconds
+ * functions to avoid excessive polling
- * *** 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)
- *
  */
-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) */

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
+ * functions to avoid excessive polling
- * *** 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)
- *
  */
-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: */
 

toxcore/network.c

@@ -25,6 +25,10 @@
 #include "config.h"
 #endif
 
+#ifndef WIN32
+#include <errno.h>
+#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 to avoid excessive polling
- * *** 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)
- *
  */
-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)
+    if (s->sendqueue_length > 0)
-        FD_SET(sock, &writefds);
+        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),
+                       strerror(errno), FD_ISSET(s->sock, &readfds), FD_ISSET(s->sock, &writefds),
-                       FD_ISSET(sock, &exceptfds));
+                       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',
+        int written = snprintf(logbuffer, sizeof(logbuffer), "[%2u] %s %3hu%c %s:%hu (%u: %s) | %04x%04x\n",
-                 ip_ntoa(&ip_port->ip), ntohs(ip_port->port), errno,
+                 buffer[0], message, (buflen < 999 ? (uint16_t)buflen : 999), 'E',
-                 strerror(errno), buflen > 4 ? ntohl(*(uint32_t *)&buffer[1]) : 0,
+                 ip_ntoa(&ip_port->ip), port, errno, strerror(errno), data[0], data[1]);
-                 (buflen > 7) ? ntohl(*(uint32_t *)(&buffer[5])) : 0);
+    }
     else if ((res > 0) && ((size_t)res <= buflen))
-        snprintf(logbuffer, sizeof(logbuffer), "[%2u] %s %3u%c %s:%u (%d: %s) | %04x%04x\n",
+        snprintf(logbuffer, sizeof(logbuffer), "[%2u] %s %3zu%c %s:%hu (%u: %s) | %04x%04x\n",
-                 buffer[0], message, res < 999 ? res : 999, (size_t)res < buflen ? '<' : '=',
+                 buffer[0], message, (res < 999 ? (size_t)res : 999), ((size_t)res < buflen ? '<' : '='),
-                 ip_ntoa(&ip_port->ip), ntohs(ip_port->port), 0,
+                 ip_ntoa(&ip_port->ip), port, 0, errmsg_ok, data[0], data[1]);
-                 "OK", buflen > 4 ? ntohl(*(uint32_t *)&buffer[1]) : 0,
-                 (buflen > 7) ? ntohl(*(uint32_t *)(&buffer[5])) : 0);
     else /* empty or overwrite */
-        snprintf(logbuffer, sizeof(logbuffer), "[%2u] %s %u%c%u %s:%u (%d: %s) | %04x%04x\n",
+        snprintf(logbuffer, sizeof(logbuffer), "[%2u] %s %zu%c%zu %s:%hu (%u: %s) | %04x%04x\n",
-                 buffer[0], message, res, !res ? '!' : '>', buflen,
+                 buffer[0], message, (size_t)res, (!res ? '!' : '>'), buflen,
-                 ip_ntoa(&ip_port->ip), ntohs(ip_port->port), 0,
+                 ip_ntoa(&ip_port->ip), port, 0, errmsg_ok, data[0], data[1]);
-                 "OK", buflen > 4 ? ntohl(*(uint32_t *)&buffer[1]) : 0,
-                 (buflen > 7) ? ntohl(*(uint32_t *)(&buffer[5])) : 0);
 
     logbuffer[sizeof(logbuffer) - 1] = 0;
     loglog(logbuffer);

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
+ * functions to avoid excessive polling
- * *** 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)
- *
  */
-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.

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
+ * functions to avoid excessive polling
- * *** 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)
- *
  */
-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: */

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  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: */