Add some tests for ping_array.

No timeout test here yet, because we don't yet have the ability to manipulate time at will, so we would have to actually sleep.
TokTok · Aug 26, 2018 · d4a4fb3 · d4a4fb3
1 parent 64ddb7f
commit d4a4fb3
Show file tree

Hide file tree

Showing 9 changed files with 218 additions and 73 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -362,6 +362,7 @@ unit_test(toxav ring_buffer)
 unit_test(toxav rtp)
 unit_test(toxcore crypto_core)
 unit_test(toxcore mono_time)
+unit_test(toxcore ping_array)
 unit_test(toxcore util)
 
 ################################################################################

diff --git a/other/astyle/format-source b/other/astyle/format-source
@@ -49,15 +49,25 @@ if grep '<unresolved>' */*.h; then
   exit 1
 fi
 
+CC_SOURCES=`find . '(' -name '*.cc' ')'`
+CC_SOURCES="$CC_SOURCES toxcore/ping_array.c"
+
+for bin in clang-format-6.0 clang-format-5.0 clang-format; do
+  if which "$bin"; then
+    "$bin" -i -style='{BasedOnStyle: Google, ColumnLimit: 100}' $CC_SOURCES
+    break
+  fi
+done
+
 FIND="find ."
-FIND="$FIND '(' -name '*.[ch]' -or -name '*.cpp' ')'"
+FIND="$FIND '(' -name '*.[ch]' ')'"
 FIND="$FIND -and -not -name '*.api.h'"
 FIND="$FIND -and -not -wholename './super_donators/*'"
 FIND="$FIND -and -not -wholename './third_party/*'"
 FIND="$FIND -and -not -wholename './toxencryptsave/crypto_pwhash*'"
 
-SOURCES=`eval "$FIND"`
+C_SOURCES=`eval "$FIND"`
 
-$ASTYLE -n --options=other/astyle/astylerc $SOURCES
+$ASTYLE -n --options=other/astyle/astylerc $C_SOURCES
 
 git diff --exit-code
diff --git a/testing/random_testing.cc b/testing/random_testing.cc
@@ -283,7 +283,8 @@ bool attempt_action(Global_State *toxes, std::mt19937 *rng) {
 int main() {
   std::vector<Action> const actions = {
       {
-          10, "creates a new conference",
+          10,
+          "creates a new conference",
           [](Local_State const &state) {
             return tox_conference_get_chatlist_size(state.tox()) < MAX_CONFERENCES_PER_USER;
           },
@@ -294,23 +295,24 @@ int main() {
           },
       },
       {
-          10, "invites a random friend to a conference",
+          10,
+          "invites a random friend to a conference",
           [](Local_State const &state) {
             return tox_conference_get_chatlist_size(state.tox()) != 0;
           },
           [](Local_State *state, Random *rnd, std::mt19937 *rng) {
             size_t chat_count = tox_conference_get_chatlist_size(state->tox());
             assert(chat_count != 0);  // Condition above.
             TOX_ERR_CONFERENCE_INVITE err;
-            tox_conference_invite(
-                state->tox(), rnd->friend_selector(*rng),
-                state->next_invite % chat_count, &err);
+            tox_conference_invite(state->tox(), rnd->friend_selector(*rng),
+                                  state->next_invite % chat_count, &err);
             state->next_invite++;
             assert(err == TOX_ERR_CONFERENCE_INVITE_OK);
           },
       },
       {
-          10, "deletes the last conference",
+          10,
+          "deletes the last conference",
           [](Local_State const &state) {
             return tox_conference_get_chatlist_size(state.tox()) != 0;
           },
@@ -322,7 +324,8 @@ int main() {
           },
       },
       {
-          10, "sends a message to the last conference",
+          10,
+          "sends a message to the last conference",
           [](Local_State const &state) {
             return tox_conference_get_chatlist_size(state.tox()) != 0;
           },
@@ -344,7 +347,9 @@ int main() {
           },
       },
       {
-          10, "changes their name", [](Local_State const &state) { return true; },
+          10,
+          "changes their name",
+          [](Local_State const &state) { return true; },
           [](Local_State *state, Random *rnd, std::mt19937 *rng) {
             std::vector<uint8_t> name(rnd->name_length_selector(*rng));
             for (uint8_t &byte : name) {
@@ -359,7 +364,9 @@ int main() {
           },
       },
       {
-          10, "sets their name to empty", [](Local_State const &state) { return true; },
+          10,
+          "sets their name to empty",
+          [](Local_State const &state) { return true; },
           [](Local_State *state, Random *rnd, std::mt19937 *rng) {
             TOX_ERR_SET_INFO err;
             tox_self_set_name(state->tox(), nullptr, 0, &err);

diff --git a/toxcore/BUILD.bazel b/toxcore/BUILD.bazel
@@ -113,6 +113,15 @@ cc_library(
     deps = [":network"],
 )
 
+cc_test(
+    name = "ping_array_test",
+    srcs = ["ping_array_test.cc"],
+    deps = [
+        ":ping_array",
+        "@com_google_googletest//:gtest_main",
+    ],
+)
+
 cc_library(
     name = "DHT",
     srcs = [

diff --git a/toxcore/crypto_core_test.cc b/toxcore/crypto_core_test.cc
@@ -10,8 +10,10 @@ enum {
   /**
    * The size of the arrays to compare. This was chosen to take around 2000
    * CPU clocks on x86_64.
+   *
+   * This is 1MiB.
    */
-  CRYPTO_TEST_MEMCMP_SIZE = 1024 * 1024,  // 1 MiB
+  CRYPTO_TEST_MEMCMP_SIZE = 1024 * 1024,
   /**
    * The number of times we run memcmp in the test.
    *

diff --git a/toxcore/ping_array.api.h b/toxcore/ping_array.api.h
@@ -22,10 +22,15 @@
  * You should have received a copy of the GNU General Public License
  * along with Tox.  If not, see <http://www.gnu.org/licenses/>.
  */
-#ifndef PING_ARRAY_H
-#define PING_ARRAY_H
+#ifndef C_TOXCORE_TOXCORE_PING_ARRAY_H
+#define C_TOXCORE_TOXCORE_PING_ARRAY_H
 
-#include "network.h"
+#include <stddef.h>
+#include <stdint.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
 %}
 
 class mono_Time { struct this; }
@@ -36,11 +41,11 @@ struct this;
 
 /**
  * Initialize a Ping_Array.
- * size represents the total size of the array and should be a power of 2.
- * timeout represents the maximum timeout in seconds for the entry.
  *
- * return 0 on success.
- * return -1 on failure.
+ * @param size represents the total size of the array and should be a power of 2.
+ * @param timeout represents the maximum timeout in seconds for the entry.
+ *
+ * @return 0 on success, -1 on failure.
  */
 static this new(uint32_t size, uint32_t timeout);
 
@@ -52,8 +57,7 @@ void kill();
 /**
  * Add a data with length to the Ping_Array list and return a ping_id.
  *
- * return ping_id on success.
- * return 0 on failure.
+ * @return ping_id on success, 0 on failure.
  */
 uint64_t add(const mono_Time::this *mono_time, const uint8_t *data, uint32_t length);
 
@@ -62,13 +66,16 @@ uint64_t add(const mono_Time::this *mono_time, const uint8_t *data, uint32_t len
  *
  * On success, copies the data into data of length,
  *
- * return length of data copied on success.
- * return -1 on failure.
+ * @return length of data copied on success, -1 on failure.
  */
 int32_t check(const mono_Time::this *mono_time, uint8_t[length] data, uint64_t ping_id);
 
 }
 
 %{
+#ifdef __cplusplus
+}  // extern "C"
 #endif
+
+#endif  // C_TOXCORE_TOXCORE_PING_ARRAY_H
 %}
diff --git a/toxcore/ping_array.c b/toxcore/ping_array.c
@@ -34,7 +34,6 @@
 #include "mono_time.h"
 #include "util.h"
 
-
 typedef struct Ping_Array_Entry {
     void *data;
     uint32_t length;
@@ -46,25 +45,23 @@ struct Ping_Array {
     Ping_Array_Entry *entries;
 
     uint32_t last_deleted; /* number representing the next entry to be deleted. */
-    uint32_t last_added; /* number representing the last entry to be added. */
-    uint32_t total_size; /* The length of entries */
-    uint32_t timeout; /* The timeout after which entries are cleared. */
+    uint32_t last_added;   /* number representing the last entry to be added. */
+    uint32_t total_size;   /* The length of entries */
+    uint32_t timeout;      /* The timeout after which entries are cleared. */
 };
 
-/* Initialize a Ping_Array.
- * size represents the total size of the array and should be a power of 2.
- * timeout represents the maximum timeout in seconds for the entry.
- *
- * return 0 on success.
- * return -1 on failure.
- */
 Ping_Array *ping_array_new(uint32_t size, uint32_t timeout)
 {
     if (size == 0 || timeout == 0) {
         return nullptr;
     }
 
-    Ping_Array *empty_array = (Ping_Array *)calloc(1, sizeof(Ping_Array));
+    if ((size & (size - 1)) != 0) {
+        // Not a power of 2.
+        return nullptr;
+    }
+
+    Ping_Array *const empty_array = (Ping_Array *)calloc(1, sizeof(Ping_Array));
 
     if (empty_array == nullptr) {
         return nullptr;
@@ -86,19 +83,15 @@ Ping_Array *ping_array_new(uint32_t size, uint32_t timeout)
 
 static void clear_entry(Ping_Array *array, uint32_t index)
 {
+    const Ping_Array_Entry empty = {nullptr};
     free(array->entries[index].data);
-    array->entries[index].data = nullptr;
-    array->entries[index].length = 0;
-    array->entries[index].time = 0;
-    array->entries[index].ping_id = 0;
+    array->entries[index] = empty;
 }
 
-/* Free all the allocated memory in a Ping_Array.
- */
 void ping_array_kill(Ping_Array *array)
 {
     while (array->last_deleted != array->last_added) {
-        uint32_t index = array->last_deleted % array->total_size;
+        const uint32_t index = array->last_deleted % array->total_size;
         clear_entry(array, index);
         ++array->last_deleted;
     }
@@ -112,7 +105,7 @@ void ping_array_kill(Ping_Array *array)
 static void ping_array_clear_timedout(Ping_Array *array, const Mono_Time *mono_time)
 {
     while (array->last_deleted != array->last_added) {
-        uint32_t index = array->last_deleted % array->total_size;
+        const uint32_t index = array->last_deleted % array->total_size;
 
         if (!mono_time_is_timeout(mono_time, array->entries[index].time, array->timeout)) {
             break;
@@ -123,15 +116,11 @@ static void ping_array_clear_timedout(Ping_Array *array, const Mono_Time *mono_t
     }
 }
 
-/* Add a data with length to the Ping_Array list and return a ping_id.
- *
- * return ping_id on success.
- * return 0 on failure.
- */
-uint64_t ping_array_add(Ping_Array *array, const Mono_Time *mono_time, const uint8_t *data, uint32_t length)
+uint64_t ping_array_add(Ping_Array *array, const Mono_Time *mono_time, const uint8_t *data,
+                        uint32_t length)
 {
     ping_array_clear_timedout(array, mono_time);
-    uint32_t index = array->last_added % array->total_size;
+    const uint32_t index = array->last_added % array->total_size;
 
     if (array->entries[index].data != nullptr) {
         array->last_deleted = array->last_added - array->total_size;
@@ -161,21 +150,14 @@ uint64_t ping_array_add(Ping_Array *array, const Mono_Time *mono_time, const uin
     return ping_id;
 }
 
-
-/* Check if ping_id is valid and not timed out.
- *
- * On success, copies the data into data of length,
- *
- * return length of data copied on success.
- * return -1 on failure.
- */
-int32_t ping_array_check(Ping_Array *array, const Mono_Time *mono_time, uint8_t *data, size_t length, uint64_t ping_id)
+int32_t ping_array_check(Ping_Array *array, const Mono_Time *mono_time, uint8_t *data,
+                         size_t length, uint64_t ping_id)
 {
     if (ping_id == 0) {
         return -1;
     }
 
-    uint32_t index = ping_id % array->total_size;
+    const uint32_t index = ping_id % array->total_size;
 
     if (array->entries[index].ping_id != ping_id) {
         return -1;
@@ -189,12 +171,13 @@ int32_t ping_array_check(Ping_Array *array, const Mono_Time *mono_time, uint8_t
         return -1;
     }
 
+    // TODO(iphydf): This can't happen? If it indeed can't, turn it into an assert.
     if (array->entries[index].data == nullptr) {
         return -1;
     }
 
     memcpy(data, array->entries[index].data, array->entries[index].length);
-    uint32_t len = array->entries[index].length;
+    const uint32_t len = array->entries[index].length;
     clear_entry(array, index);
     return len;
 }
diff --git a/toxcore/ping_array.h b/toxcore/ping_array.h
@@ -21,10 +21,15 @@
  * You should have received a copy of the GNU General Public License
  * along with Tox.  If not, see <http://www.gnu.org/licenses/>.
  */
-#ifndef PING_ARRAY_H
-#define PING_ARRAY_H
+#ifndef C_TOXCORE_TOXCORE_PING_ARRAY_H
+#define C_TOXCORE_TOXCORE_PING_ARRAY_H
 
-#include "network.h"
+#include <stddef.h>
+#include <stdint.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
 
 #ifndef MONO_TIME_DEFINED
 #define MONO_TIME_DEFINED
@@ -38,11 +43,11 @@ typedef struct Ping_Array Ping_Array;
 
 /**
  * Initialize a Ping_Array.
- * size represents the total size of the array and should be a power of 2.
- * timeout represents the maximum timeout in seconds for the entry.
  *
- * return 0 on success.
- * return -1 on failure.
+ * @param size represents the total size of the array and should be a power of 2.
+ * @param timeout represents the maximum timeout in seconds for the entry.
+ *
+ * @return 0 on success, -1 on failure.
  */
 struct Ping_Array *ping_array_new(uint32_t size, uint32_t timeout);
 
@@ -54,8 +59,7 @@ void ping_array_kill(struct Ping_Array *_array);
 /**
  * Add a data with length to the Ping_Array list and return a ping_id.
  *
- * return ping_id on success.
- * return 0 on failure.
+ * @return ping_id on success, 0 on failure.
  */
 uint64_t ping_array_add(struct Ping_Array *_array, const struct Mono_Time *mono_time, const uint8_t *data,
                         uint32_t length);
@@ -65,10 +69,13 @@ uint64_t ping_array_add(struct Ping_Array *_array, const struct Mono_Time *mono_
  *
  * On success, copies the data into data of length,
  *
- * return length of data copied on success.
- * return -1 on failure.
+ * @return length of data copied on success, -1 on failure.
  */
 int32_t ping_array_check(struct Ping_Array *_array, const struct Mono_Time *mono_time, uint8_t *data, size_t length,
                          uint64_t ping_id);
 
+#ifdef __cplusplus
+}  // extern "C"
 #endif
+
+#endif  // C_TOXCORE_TOXCORE_PING_ARRAY_H