Merge pull request #790 from nurupo/tox-wait-fix

Some tox_wait_* improvements

toxcore/Messenger.c

@@ -2342,19 +2342,24 @@ void do_messenger(Messenger *m)
 /*
  * functions to avoid excessive polling
  */
-int wait_prepare_messenger(Messenger *m, uint8_t *data, uint16_t *lenptr)
+size_t wait_data_size()
 {
-    return networking_wait_prepare(m->net, sendqueue_total(m->net_crypto->lossless_udp), data, lenptr);
+    return networking_wait_data_size();
 }
 
-int wait_execute_messenger(Messenger *m, uint8_t *data, uint16_t len, uint16_t milliseconds)
+int wait_prepare_messenger(Messenger *m, uint8_t *data)
 {
-    return networking_wait_execute(data, len, milliseconds);
-};
+    return networking_wait_prepare(m->net, sendqueue_total(m->net_crypto->lossless_udp), data);
+}
 
-void wait_cleanup_messenger(Messenger *m, uint8_t *data, uint16_t len)
+int wait_execute_messenger(uint8_t *data, long seconds, long microseconds)
 {
-    networking_wait_cleanup(m->net, data, len);
+    return networking_wait_execute(data, seconds, microseconds);
+}
+
+int wait_cleanup_messenger(Messenger *m, uint8_t *data)
+{
+    return networking_wait_cleanup(m->net, data);
 }
 
 /* new messenger format for load/save, more robust and forward compatible */

toxcore/Messenger.h

@@ -715,9 +715,10 @@ void do_messenger(Messenger *m);
 /*
  * functions to avoid excessive polling
  */
-int wait_prepare_messenger(Messenger *m, uint8_t *data, uint16_t *lenptr);
-int wait_execute_messenger(Messenger *m, uint8_t *data, uint16_t len, uint16_t milliseconds);
-void wait_cleanup_messenger(Messenger *m, uint8_t *data, uint16_t len);
+size_t wait_data_size();
+int wait_prepare_messenger(Messenger *m, uint8_t *data);
+int wait_execute_messenger(uint8_t *data, long seconds, long microseconds);
+int wait_cleanup_messenger(Messenger *m, uint8_t *data);
 
 /* SAVING AND LOADING FUNCTIONS: */
 

toxcore/network.c

@@ -315,17 +315,17 @@ typedef struct {
     uint64_t send_fail_eagain;
 } select_info;
 
-int networking_wait_prepare(Networking_Core *net, uint32_t sendqueue_length, uint8_t *data, uint16_t *lenptr)
+size_t networking_wait_data_size()
 {
-    if ((data == NULL) || !lenptr || (*lenptr < sizeof(select_info))) {
-        if (lenptr) {
-            *lenptr = sizeof(select_info);
-            return 0;
-        } else
-            return -1;
+    return sizeof(select_info);
+}
+
+int networking_wait_prepare(Networking_Core *net, uint32_t sendqueue_length, uint8_t *data)
+{
+    if (data == NULL) {
+        return 0;
     }
 
-    *lenptr = sizeof(select_info);
     select_info *s = (select_info *)data;
     s->sock = net->sock;
     s->sendqueue_length = sendqueue_length;
@@ -335,25 +335,41 @@ int networking_wait_prepare(Networking_Core *net, uint32_t sendqueue_length, uin
     return 1;
 }
 
-int networking_wait_execute(uint8_t *data, uint16_t len, uint16_t milliseconds)
+/* *** Function MUSTN'T poll. ***
+* The function mustn't modify anything at all, so it can be called completely
+* asynchronously without any worry.
+*/
+int networking_wait_execute(uint8_t *data, long seconds, long microseconds)
 {
     /* WIN32: supported since Win2K, but might need some adjustements */
     /* UNIX: this should work for any remotely Unix'ish system */
 
+    if (data == NULL) {
+        return 0;
+    }
+
     select_info *s = (select_info *)data;
 
     /* add only if we had a failed write */
     int writefds_add = 0;
 
+    /* if send_fail_eagain is set, that means that socket's buffer was full and couldn't fit data we tried to send,
+    *  so this is the only case when we need to know when the socket becomes write-ready, i.e. socket's buffer gets
+    *  some free space for us to put data to be sent in, but select will tell us that the socket is writable even
+    *  if we can fit a small part of our data (say 1 byte), so we wait some time, in hope that large enough chunk
+    *  of socket's buffer will be available (at least that's how I understand intentions of the previous author of
+    *  that code)
+    */
     if (s->send_fail_eagain != 0) {
         // current_time(): microseconds
         uint64_t now = current_time();
 
         /* s->sendqueue_length: might be used to guess how long we keep checking */
-        /* for now, threshold is hardcoded to 500ms, too long for a really really
+        /* for now, threshold is hardcoded to 250ms, too long for a really really
          * fast link, but too short for a sloooooow link... */
-        if (now - s->send_fail_eagain < 500000)
+        if (now - s->send_fail_eagain < 250000) {
             writefds_add = 1;
+        }
     }
 
     int nfds = 1 + s->sock;
@@ -366,27 +382,34 @@ int networking_wait_execute(uint8_t *data, uint16_t len, uint16_t milliseconds)
     fd_set writefds;
     FD_ZERO(&writefds);
 
-    if (writefds_add)
+    if (writefds_add) {
         FD_SET(s->sock, &writefds);
+    }
 
     fd_set exceptfds;
     FD_ZERO(&exceptfds);
     FD_SET(s->sock, &exceptfds);
 
     struct timeval timeout;
-    timeout.tv_sec = 0;
-    timeout.tv_usec = milliseconds * 1000;
+    struct timeval *timeout_ptr = &timeout;
+
+    if (seconds < 0 || microseconds < 0) {
+        timeout_ptr = NULL;
+    } else {
+        timeout.tv_sec = seconds;
+        timeout.tv_usec = microseconds;
+    }
 
 #ifdef LOGGING
     errno = 0;
 #endif
     /* returns -1 on error, 0 on timeout, the socket on activity */
-    int res = select(nfds, &readfds, &writefds, &exceptfds, &timeout);
+    int res = select(nfds, &readfds, &writefds, &exceptfds, timeout_ptr);
 #ifdef LOGGING
 
     /* only dump if not timeout */
     if (res) {
-        sprintf(logbuffer, "select(%d): %d (%d, %s) - %d %d %d\n", milliseconds, res, errno,
+        sprintf(logbuffer, "select(%d, %d): %d (%d, %s) - %d %d %d\n", microseconds, seconds, res, errno,
                 strerror(errno), FD_ISSET(s->sock, &readfds), FD_ISSET(s->sock, &writefds),
                 FD_ISSET(s->sock, &exceptfds));
         loglog(logbuffer);
@@ -394,18 +417,26 @@ int networking_wait_execute(uint8_t *data, uint16_t len, uint16_t milliseconds)
 
 #endif
 
-    if (FD_ISSET(s->sock, &writefds))
+    if (FD_ISSET(s->sock, &writefds)) {
         s->send_fail_reset = 1;
+    }
 
-    return res > 0 ? 1 : 0;
+    return res > 0 ? 2 : 1;
 }
 
-void networking_wait_cleanup(Networking_Core *net, uint8_t *data, uint16_t len)
+int networking_wait_cleanup(Networking_Core *net, uint8_t *data)
 {
+    if (data == NULL) {
+        return 0;
+    }
+
     select_info *s = (select_info *)data;
 
-    if (s->send_fail_reset)
+    if (s->send_fail_reset) {
         net->send_fail_eagain = 0;
+    }
+    
+    return 1;
 }
 
 uint8_t at_startup_ran = 0;

toxcore/network.h

@@ -312,9 +312,10 @@ void networking_poll(Networking_Core *net);
 /*
  * functions to avoid excessive polling
  */
-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);
-void networking_wait_cleanup(Networking_Core *net, uint8_t *data, uint16_t len);
+size_t networking_wait_data_size();
+int networking_wait_prepare(Networking_Core *net, uint32_t sendqueue_length, uint8_t *data);
+int networking_wait_execute(uint8_t *data, long seconds, long microseconds);
+int networking_wait_cleanup(Networking_Core *net, uint8_t *data);
 
 /* Initialize networking.
  * bind to ip and port.

toxcore/tox.c

@@ -767,22 +767,27 @@ void tox_do(Tox *tox)
 /*
  * functions to avoid excessive polling
  */
-int tox_wait_prepare(Tox *tox, uint8_t *data, uint16_t *lenptr)
+
+size_t tox_wait_data_size()
 {
-    Messenger *m = tox;
-    return wait_prepare_messenger(m, data, lenptr);
+    return wait_data_size();
 }
 
-int tox_wait_execute(Tox *tox, uint8_t *data, uint16_t len, uint16_t milliseconds)
+int tox_wait_prepare(Tox *tox, uint8_t *data)
 {
     Messenger *m = tox;
-    return wait_execute_messenger(m, data, len, milliseconds);
+    return wait_prepare_messenger(m, data);
 }
 
-void tox_wait_cleanup(Tox *tox, uint8_t *data, uint16_t len)
+int tox_wait_execute(uint8_t *data, long seconds, long microseconds)
+{
+    return wait_execute_messenger(data, seconds, microseconds);
+}
+
+int tox_wait_cleanup(Tox *tox, uint8_t *data)
 {
     Messenger *m = tox;
-    wait_cleanup_messenger(m, data, len);
+    return wait_cleanup_messenger(m, data);
 }
 
 /* SAVING AND LOADING FUNCTIONS: */

toxcore/tox.h

@@ -662,37 +662,43 @@ void tox_kill(Tox *tox);
 void tox_do(Tox *tox);
 
 /*
- * tox_wait_prepare(): function should be called under lock
+ * tox_wait_data_size():
+ *
+ *  returns a size of data buffer to allocate. the size is constant.
+ *
+ * tox_wait_prepare(): function should be called under lock every time we want to call tox_wait_execute()
  * Prepares the data required to call tox_wait_execute() asynchronously
  *
- * data[] is reserved and kept by the caller
- * *lenptr is in/out: in = reserved data[], out = required data[]
+ * data[] should be of at least tox_wait_data_size() size and it's reserved and kept by the caller
+ * Use that data[] to call tox_wait_execute()
  *
  *  returns  1 on success
- *  returns  0 if *lenptr is insufficient
- *  returns -1 if lenptr is NULL
+ *  returns  0 if data was NULL
  *
  *
  * 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.
+ * Waits for something to happen on the socket for up to seconds seconds and mircoseconds microseconds.
+ * mircoseconds should be between 0 and 999999.
+ * If you set either or both seconds and microseconds to negatives, it will block indefinetly until there
+ * is an activity.
  *
- *  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
+ *  returns  2 if there is socket activity (i.e. tox_do() should be called)
+ *  returns  1 if the timeout was reached (tox_do() should be called anyway. it's advised to call it at least
+ *             once per second)
+ *  returns  0 if data was NULL
  *
  *
- * tox_wait_cleanup(): function should be called under lock
+ * tox_wait_cleanup(): function should be called under lock,  every time tox_wait_execute() finishes
  * Stores results from tox_wait_execute().
  *
- * data[]/len shall be the exact same as given to tox_wait_execute()
+ *  returns  1 on success
+ *  returns  0 if data was NULL
  *
  */
-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);
-void tox_wait_cleanup(Tox *tox, uint8_t *data, uint16_t len);
+size_t tox_wait_data_size();
+int tox_wait_prepare(Tox *tox, uint8_t *data);
+int tox_wait_execute(uint8_t *data, long seconds, long microseconds);
+int tox_wait_cleanup(Tox *tox, uint8_t *data);
 
 
 /* SAVING AND LOADING FUNCTIONS: */