Tox loop: rework some logic

CMakeLists.txt

@@ -434,7 +434,6 @@ if(BUILD_TOXAV)
   auto_test(toxav_many)
 endif()
 
-
 ################################################################################
 #
 # :: Bootstrap daemon

auto_tests/tox_loop_test.c

@@ -1,16 +1,14 @@
-
-
-#include "helpers.h"
-
-#include "../toxcore/tox.h"
-
 #include <check.h>
 #include <pthread.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <time.h>
 #include <unistd.h>
 
+#include "../toxcore/tox.h"
+
+#include "helpers.h"
+
 #define TCP_RELAY_PORT 33448
 /* The Travis-CI container responds poorly to ::1 as a localhost address
  * You're encouraged to -D FORCE_TESTS_IPV6 on a local test  */
@@ -50,7 +48,7 @@ void *tox_loop_worker(void *data)
 START_TEST(test_tox_loop)
 {
     pthread_t worker, worker_tcp;
-    struct Tox_Options opts;
+    struct Tox_Options *opts = tox_options_new(NULL);
     loop_test userdata;
     uint8_t dpk[TOX_PUBLIC_KEY_SIZE];
     int retval;
@@ -59,21 +57,20 @@ START_TEST(test_tox_loop)
     userdata.stop_count = 0;
     pthread_mutex_init(&userdata.mutex, NULL);
 
-    tox_options_default(&opts);
-    opts.tcp_port = TCP_RELAY_PORT;
-    userdata.tox = tox_new(&opts, 0);
+    tox_options_set_tcp_port(opts, TCP_RELAY_PORT);
+    userdata.tox = tox_new(opts, NULL);
     tox_callback_loop_begin(userdata.tox, tox_loop_cb_start);
     tox_callback_loop_end(userdata.tox, tox_loop_cb_stop);
     pthread_create(&worker, NULL, tox_loop_worker, &userdata);
 
     tox_self_get_dht_id(userdata.tox, dpk);
 
-    tox_options_default(&opts);
+    tox_options_default(opts);
     loop_test userdata_tcp;
     userdata_tcp.start_count = 0;
     userdata_tcp.stop_count = 0;
     pthread_mutex_init(&userdata_tcp.mutex, NULL);
-    userdata_tcp.tox = tox_new(&opts, 0);
+    userdata_tcp.tox = tox_new(opts, NULL);
     tox_callback_loop_begin(userdata_tcp.tox, tox_loop_cb_start);
     tox_callback_loop_end(userdata_tcp.tox, tox_loop_cb_stop);
     pthread_create(&worker_tcp, NULL, tox_loop_worker, &userdata_tcp);

toxcore/Messenger.h

@@ -273,8 +273,8 @@ struct Messenger {
     unsigned int last_connection_status;
 
     bool loop_run;
-    void (*loop_begin_cb)(struct Messenger *tox, void *user_data);
-    void (*loop_end_cb)(struct Messenger *tox, void *user_data);
+    void (*loop_begin_cb)(struct Messenger *m, void *user_data);
+    void (*loop_end_cb)(struct Messenger *m, void *user_data);
 
     Messenger_Options options;
 };

toxcore/tox.api.h

@@ -838,10 +838,18 @@ void iterate(any user_data);
  * Error codes for $loop().
  */
 error for loop {
+  /**
+   * Invalid arguments passed.
+   */
+  BAD_ARGS,
   /**
    * Failed running select().
    */
   SELECT,
+  /**
+   * Failed getting sockets file descriptors.
+   */
+  GET_FDS,
 }
 
 

toxcore/tox.c

@@ -472,62 +472,78 @@ void tox_iterate(Tox *tox, void *user_data)
 }
 
 /**
- * Gathers a list of every network FD activity is expected on
- * @param sockets an array of size tox_fd_count().
+ * Gathers a list of every network file descriptor,
+ * where an activity is expected on.
+ *
+ * @param sockets a pointer to an array (the pointed array can be NULL).
+ * @param sockets_num the number of current known sockets (will be updated by the funciton).
+ *
+ * @return false if errors occurred, true otherwise.
  */
-uint32_t tox_fds(Tox *tox, sock_t **sockets, uint32_t max_sockets)
+bool tox_fds(Messenger *m, sock_t **sockets, uint32_t *sockets_num)
 {
-    Messenger *m = tox;
-    uint32_t i, count, fdcount;
+    if ((m == NULL) || (sockets == NULL) || (sockets_num == NULL)) {
+        return false;
+    }
+
+    uint32_t i, fdcount;
 
-    count = 0;
     fdcount = 1 + m->net_crypto->tcp_c->tcp_connections_length;
 
-    if (fdcount != max_sockets) {
+    if ((fdcount != *sockets_num) || (*sockets == NULL)) {
         sock_t *tmp_sockets = (sock_t *) realloc(*sockets, fdcount * sizeof(sock_t));
 
-        if (tmp_sockets != NULL) {
+        if (tmp_sockets == NULL) {
+            return false;
+        } else {
             *sockets = tmp_sockets;
-            max_sockets = fdcount;
+            *sockets_num = fdcount;
         }
     }
 
-    if (max_sockets > 0) {
-        (*sockets)[count] = m->net->sock;
-        count++;
-        max_sockets--;
-    }
+    (*sockets)[0] = m->net->sock;
 
     TCP_Connections *conns = m->net_crypto->tcp_c;
+    i = 0;
 
-    for (i = 0; i < conns->tcp_connections_length; i++) {
-        if (max_sockets == 0) {
-            break;
-        }
-
+    while ((i < (fdcount - 1)) && (i < conns->tcp_connections_length)) {
         TCP_con *conn = &conns->tcp_connections[i];
-        (*sockets)[count] = conn->connection->sock;
-        count++;
-        max_sockets--;
+        (*sockets)[++i] = conn->connection->sock;
     }
 
-    return fdcount;
+    return true;
 }
 
 void tox_callback_loop_begin(Tox *tox, tox_loop_begin_cb *callback)
 {
+    if (tox == NULL) {
+        return;
+    }
+
     Messenger *m = tox;
     m->loop_begin_cb = callback;
 }
 
 void tox_callback_loop_end(Tox *tox, tox_loop_end_cb *callback)
 {
+    if (tox == NULL) {
+        return;
+    }
+
     Messenger *m = tox;
     m->loop_end_cb = callback;
 }
 
 bool tox_loop(Tox *tox, void *user_data, TOX_ERR_LOOP *error)
 {
+    if (tox == NULL) {
+        if (error != NULL) {
+            *error = TOX_ERR_LOOP_BAD_ARGS;
+        }
+
+        return false;
+    }
+
     Messenger *m = tox;
     uint32_t fdcount = 0;
     sock_t *fdlist = NULL;
@@ -548,7 +564,23 @@ bool tox_loop(Tox *tox, void *user_data, TOX_ERR_LOOP *error)
         maxfd = 0;
         FD_ZERO(&readable);
 
-        fdcount = tox_fds(tox, &fdlist, fdcount);
+        // TODO(cleverca22): is it a good idea to reuse previous fdlist when
+        //                   fdcount!=0 && tox_fds()==false?
+        if ((fdcount == 0) && (!tox_fds(m, &fdlist, &fdcount))) {
+            // We must stop because maxfd won't be set.
+            if (error != NULL) {
+                *error = TOX_ERR_LOOP_GET_FDS;
+            }
+
+            // TODO(cleverca22): should we call loop_end_cb() on error?
+            if (m->loop_end_cb) {
+                m->loop_end_cb(tox, user_data);
+            }
+
+            free(fdlist);
+
+            return false;
+        }
 
         for (i = 0; i < fdcount; i++) {
             FD_SET(fdlist[i], &readable);
@@ -562,7 +594,8 @@ bool tox_loop(Tox *tox, void *user_data, TOX_ERR_LOOP *error)
 
         timeout.tv_sec = 0;
 
-        timeout.tv_usec = tox_iteration_interval(tox) * 1000 * 2; // TODO(cleverca22): use a longer timeout.
+        // TODO(cleverca22): use a longer timeout.
+        timeout.tv_usec = tox_iteration_interval(tox) * 1000 * 2;
 
         if (m->loop_end_cb) {
             m->loop_end_cb(tox, user_data);

toxcore/tox.h

@@ -993,11 +993,21 @@ typedef enum TOX_ERR_LOOP {
      */
     TOX_ERR_LOOP_OK,
 
+    /**
+     * Invalid arguments passed.
+     */
+    TOX_ERR_LOOP_BAD_ARGS,
+
     /**
      * Failed running select().
      */
     TOX_ERR_LOOP_SELECT,
 
+    /**
+     * Failed getting sockets file descriptors.
+     */
+    TOX_ERR_LOOP_GET_FDS,
+
 } TOX_ERR_LOOP;