refactor: Don't expose Tox_System in the public API

It makes no sense to include it in the public API as clients can't make
any meaningful use of it via public API, it can only be used if one also
includes other internal/private headers that we don't install.

It's used only in the testing code, which has access to the internal
headers.

Fixes #2739, at least to some degree. I decided against moving things to
a separate `tox_testing.h` and leaving only things in `tox_private.h`
that we are fine with clients using, as otherwise `tox_lock()` /
`tox_unlock()` would have to be moved out of `tox_private.h` to
somewhere else, but `tox_private.h` actually sounds like the right place
for them, naming-wise. So perhaps it's fine if we have things in
`tox_private.h` that we don't want clients to use.

Author: nurupo

testing/fuzzing/bootstrap_fuzz_test.cc

@@ -104,7 +104,6 @@ void TestBootstrap(Fuzz_Data &input)
 
     Ptr<Tox_Options> opts(tox_options_new(nullptr), tox_options_free);
     assert(opts != nullptr);
-    tox_options_set_operating_system(opts.get(), sys.sys.get());
 
     tox_options_set_log_callback(opts.get(),
         [](Tox *tox, Tox_Log_Level level, const char *file, uint32_t line, const char *func,
@@ -134,8 +133,12 @@ void TestBootstrap(Fuzz_Data &input)
         tox_options_set_tcp_port(opts.get(), 33445);
     }
 
+    Tox_Options_Testing tox_options_testing;
+    tox_options_testing.operating_system = sys.sys.get();
+
     Tox_Err_New error_new;
-    Tox *tox = tox_new(opts.get(), &error_new);
+    Tox_Err_New_Testing error_new_testing;
+    Tox *tox = tox_new_testing(opts.get(), &error_new, &tox_options_testing, &error_new_testing);
 
     if (tox == nullptr) {
         // It might fail, because some I/O happens in tox_new, and the fuzzer
@@ -144,6 +147,7 @@ void TestBootstrap(Fuzz_Data &input)
     }
 
     assert(error_new == TOX_ERR_NEW_OK);
+    assert(error_new_testing == TOX_ERR_NEW_TESTING_OK);
 
     uint8_t pub_key[TOX_PUBLIC_KEY_SIZE] = {0};
 

testing/fuzzing/e2e_fuzz_test.cc

@@ -138,7 +138,6 @@ void TestEndToEnd(Fuzz_Data &input)
 
     Ptr<Tox_Options> opts(tox_options_new(nullptr), tox_options_free);
     assert(opts != nullptr);
-    tox_options_set_operating_system(opts.get(), sys.sys.get());
     tox_options_set_local_discovery_enabled(opts.get(), false);
 
     tox_options_set_log_callback(opts.get(),
@@ -151,8 +150,12 @@ void TestEndToEnd(Fuzz_Data &input)
             }
         });
 
+    Tox_Options_Testing tox_options_testing;
+    tox_options_testing.operating_system = sys.sys.get();
+
     Tox_Err_New error_new;
-    Tox *tox = tox_new(opts.get(), &error_new);
+    Tox_Err_New_Testing error_new_testing;
+    Tox *tox = tox_new_testing(opts.get(), &error_new, &tox_options_testing, &error_new_testing);
 
     if (tox == nullptr) {
         // It might fail, because some I/O happens in tox_new, and the fuzzer
@@ -161,6 +164,7 @@ void TestEndToEnd(Fuzz_Data &input)
     }
 
     assert(error_new == TOX_ERR_NEW_OK);
+    assert(error_new_testing == TOX_ERR_NEW_TESTING_OK);
 
     tox_events_init(tox);
 

testing/fuzzing/fuzz_support.hh

@@ -16,6 +16,7 @@
 #include <vector>
 
 #include "../../toxcore/tox.h"
+#include "../../toxcore/tox_private.h"
 
 struct Fuzz_Data {
     static constexpr bool DEBUG = false;

testing/fuzzing/protodump.cc

@@ -195,13 +195,16 @@ void RecordBootstrap(const char *init, const char *bootstrap)
         });
 
     Tox_Err_New error_new;
+    Tox_Err_New_Testing error_new_testing;
+    Tox_Options_Testing tox_options_testing;
 
     Record_System sys1(global, 4, "tox1");  // fair dice roll
     tox_options_set_log_user_data(opts, &sys1);
-    tox_options_set_operating_system(opts, sys1.sys.get());
-    Tox *tox1 = tox_new(opts, &error_new);
+    tox_options_testing.operating_system = sys1.sys.get();
+    Tox *tox1 = tox_new_testing(opts, &error_new, &tox_options_testing, &error_new_testing);
     assert(tox1 != nullptr);
     assert(error_new == TOX_ERR_NEW_OK);
+    assert(error_new_testing == TOX_ERR_NEW_TESTING_OK);
     std::array<uint8_t, TOX_ADDRESS_SIZE> address1;
     tox_self_get_address(tox1, address1.data());
     std::array<uint8_t, TOX_PUBLIC_KEY_SIZE> pk1;
@@ -211,10 +214,11 @@ void RecordBootstrap(const char *init, const char *bootstrap)
 
     Record_System sys2(global, 5, "tox2");  // unfair dice roll
     tox_options_set_log_user_data(opts, &sys2);
-    tox_options_set_operating_system(opts, sys2.sys.get());
-    Tox *tox2 = tox_new(opts, &error_new);
+    tox_options_testing.operating_system = sys2.sys.get();
+    Tox *tox2 = tox_new_testing(opts, &error_new, &tox_options_testing, &error_new_testing);
     assert(tox2 != nullptr);
     assert(error_new == TOX_ERR_NEW_OK);
+    assert(error_new_testing == TOX_ERR_NEW_TESTING_OK);
     std::array<uint8_t, TOX_ADDRESS_SIZE> address2;
     tox_self_get_address(tox2, address2.data());
     std::array<uint8_t, TOX_PUBLIC_KEY_SIZE> pk2;

testing/fuzzing/protodump_reduce.cc

@@ -142,9 +142,11 @@ void TestEndToEnd(Fuzz_Data &input)
 
     Ptr<Tox_Options> opts(tox_options_new(nullptr), tox_options_free);
     assert(opts != nullptr);
-    tox_options_set_operating_system(opts.get(), sys.sys.get());
     tox_options_set_local_discovery_enabled(opts.get(), false);
 
+    Tox_Options_Testing tox_options_testing;
+    tox_options_testing.operating_system = sys.sys.get();
+
     tox_options_set_log_callback(opts.get(),
         [](Tox *tox, Tox_Log_Level level, const char *file, uint32_t line, const char *func,
             const char *message, void *user_data) {
@@ -156,7 +158,8 @@ void TestEndToEnd(Fuzz_Data &input)
         });
 
     Tox_Err_New error_new;
-    Tox *tox = tox_new(opts.get(), &error_new);
+    Tox_Err_New_Testing error_new_testing;
+    Tox *tox = tox_new_testing(opts.get(), &error_new, &tox_options_testing, &error_new_testing);
 
     if (tox == nullptr) {
         // It might fail, because some I/O happens in tox_new, and the fuzzer
@@ -165,6 +168,7 @@ void TestEndToEnd(Fuzz_Data &input)
     }
 
     assert(error_new == TOX_ERR_NEW_OK);
+    assert(error_new_testing == TOX_ERR_NEW_TESTING_OK);
 
     tox_events_init(tox);
 

testing/fuzzing/toxsave_fuzz_test.cc

@@ -20,14 +20,15 @@ void TestSaveDataLoading(Fuzz_Data &input)
     const size_t savedata_size = input.size();
     CONSUME_OR_RETURN(const uint8_t *savedata, input, savedata_size);
 
-    Null_System sys;
-    tox_options_set_operating_system(tox_options, sys.sys.get());
-
     // pass test data to Tox
     tox_options_set_savedata_data(tox_options, savedata, savedata_size);
     tox_options_set_savedata_type(tox_options, TOX_SAVEDATA_TYPE_TOX_SAVE);
 
-    Tox *tox = tox_new(tox_options, nullptr);
+    Tox_Options_Testing tox_options_testing;
+    Null_System sys;
+    tox_options_testing.operating_system = sys.sys.get();
+
+    Tox *tox = tox_new_testing(tox_options, nullptr, &tox_options_testing, nullptr);
     tox_options_free(tox_options);
     if (tox == nullptr) {
         // Tox save was invalid, we're finished here

toxcore/tox.c

@@ -712,7 +712,8 @@ static int tox_load(Tox *tox, const uint8_t *data, uint32_t length)
                       length - cookie_len, STATE_COOKIE_TYPE);
 }
 
-Tox *tox_new(const struct Tox_Options *options, Tox_Err_New *error)
+nullable(1, 2, 3)
+static Tox *tox_new_system(const struct Tox_Options *options, Tox_Err_New *error, const Tox_System *sys)
 {
     struct Tox_Options *default_options = nullptr;
 
@@ -736,7 +737,6 @@ Tox *tox_new(const struct Tox_Options *options, Tox_Err_New *error)
     const struct Tox_Options *const opts = options != nullptr ? options : default_options;
     assert(opts != nullptr);
 
-    const Tox_System *sys = tox_options_get_operating_system(opts);
     const Tox_System default_system = tox_default_system();
 
     if (sys == nullptr) {
@@ -1020,6 +1020,37 @@ Tox *tox_new(const struct Tox_Options *options, Tox_Err_New *error)
     return tox;
 }
 
+Tox *tox_new(const struct Tox_Options *options, Tox_Err_New *error)
+{
+    return tox_new_system(options, error, nullptr);
+}
+
+Tox *tox_new_testing(const Tox_Options *options, Tox_Err_New *error, const Tox_Options_Testing *testing, Tox_Err_New_Testing *testing_error)
+{
+    if (testing == nullptr) {
+        SET_ERROR_PARAMETER(error, TOX_ERR_NEW_NULL);
+        SET_ERROR_PARAMETER(testing_error, TOX_ERR_NEW_TESTING_NULL);
+        return nullptr;
+    }
+
+    if (testing->operating_system == nullptr) {
+        SET_ERROR_PARAMETER(error, TOX_ERR_NEW_NULL);
+        SET_ERROR_PARAMETER(testing_error, TOX_ERR_NEW_TESTING_NULL);
+        return nullptr;
+    }
+
+    const Tox_System *sys = testing->operating_system;
+
+    if (sys->rng == nullptr || sys->ns == nullptr || sys->mem == nullptr) {
+        SET_ERROR_PARAMETER(error, TOX_ERR_NEW_NULL);
+        SET_ERROR_PARAMETER(testing_error, TOX_ERR_NEW_TESTING_NULL);
+        return nullptr;
+    }
+
+    SET_ERROR_PARAMETER(testing_error, TOX_ERR_NEW_TESTING_OK);
+    return tox_new_system(options, error, sys);
+}
+
 void tox_kill(Tox *tox)
 {
     if (tox == nullptr) {

toxcore/tox.h

@@ -496,15 +496,6 @@ const char *tox_log_level_to_string(Tox_Log_Level value);
 typedef void tox_log_cb(Tox *tox, Tox_Log_Level level, const char *file, uint32_t line, const char *func,
                         const char *message, void *user_data);
 
-/**
- * @brief Operating system functions used by Tox.
- *
- * This struct is opaque and generally shouldn't be used in clients, but in
- * combination with tox_private.h, it allows tests to inject non-IO (hermetic)
- * versions of low level network, RNG, and time keeping functions.
- */
-typedef struct Tox_System Tox_System;
-
 /**
  * @brief This struct contains all the startup options for Tox.
  *
@@ -665,12 +656,6 @@ struct Tox_Options {
      */
     bool experimental_thread_safety;
 
-    /**
-     * Low level operating system functionality such as send/recv, random
-     * number generation, and memory allocation.
-     */
-    const Tox_System *operating_system;
-
     /**
      * Enable saving DHT-based group chats to Tox save data (via
      * `tox_get_savedata`). This format will change in the future, so don't rely
@@ -753,10 +738,6 @@ bool tox_options_get_experimental_thread_safety(const Tox_Options *options);
 
 void tox_options_set_experimental_thread_safety(Tox_Options *options, bool experimental_thread_safety);
 
-const Tox_System *tox_options_get_operating_system(const Tox_Options *options);
-
-void tox_options_set_operating_system(Tox_Options *options, const Tox_System *operating_system);
-
 bool tox_options_get_experimental_groups_persistence(const Tox_Options *options);
 
 void tox_options_set_experimental_groups_persistence(Tox_Options *options, bool experimental_groups_persistence);

toxcore/tox_api.c

@@ -265,14 +265,6 @@ void tox_options_set_experimental_thread_safety(
 {
     options->experimental_thread_safety = experimental_thread_safety;
 }
-const Tox_System *tox_options_get_operating_system(const Tox_Options *options)
-{
-    return options->operating_system;
-}
-void tox_options_set_operating_system(Tox_Options *options, const Tox_System *operating_system)
-{
-    options->operating_system = operating_system;
-}
 bool tox_options_get_experimental_groups_persistence(const Tox_Options *options)
 {
     return options->experimental_groups_persistence;

toxcore/tox_events.h

@@ -570,6 +570,8 @@ void tox_events_free(Tox_Events *events);
 uint32_t tox_events_bytes_size(const Tox_Events *events);
 bool tox_events_get_bytes(const Tox_Events *events, uint8_t *bytes);
 
+typedef struct Tox_System Tox_System;
+
 Tox_Events *tox_events_load(const Tox_System *sys, const uint8_t *bytes, uint32_t bytes_size);
 
 bool tox_events_equal(const Tox_System *sys, const Tox_Events *a, const Tox_Events *b);