From 331381daef28ae89bb1cb329c4b77b5718e8a936 Mon Sep 17 00:00:00 2001 From: Maxim Biro Date: Sun, 17 Mar 2024 10:43:37 -0400 Subject: [PATCH] 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. --- testing/fuzzing/bootstrap_fuzz_test.cc | 8 ++++-- testing/fuzzing/e2e_fuzz_test.cc | 8 ++++-- testing/fuzzing/fuzz_support.hh | 1 + testing/fuzzing/protodump.cc | 7 ++++-- testing/fuzzing/protodump_reduce.cc | 8 ++++-- testing/fuzzing/toxsave_fuzz_test.cc | 9 ++++--- toxcore/tox.c | 35 ++++++++++++++++++++++++-- toxcore/tox.h | 19 -------------- toxcore/tox_api.c | 8 ------ toxcore/tox_events.h | 2 ++ toxcore/tox_private.h | 19 +++++++++++--- 11 files changed, 79 insertions(+), 45 deletions(-) diff --git a/testing/fuzzing/bootstrap_fuzz_test.cc b/testing/fuzzing/bootstrap_fuzz_test.cc index f25e24319d0..4a784f846a0 100644 --- a/testing/fuzzing/bootstrap_fuzz_test.cc +++ b/testing/fuzzing/bootstrap_fuzz_test.cc @@ -104,7 +104,6 @@ void TestBootstrap(Fuzz_Data &input) Ptr 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}; diff --git a/testing/fuzzing/e2e_fuzz_test.cc b/testing/fuzzing/e2e_fuzz_test.cc index 1be228e0400..df4e03c6982 100644 --- a/testing/fuzzing/e2e_fuzz_test.cc +++ b/testing/fuzzing/e2e_fuzz_test.cc @@ -138,7 +138,6 @@ void TestEndToEnd(Fuzz_Data &input) Ptr 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); diff --git a/testing/fuzzing/fuzz_support.hh b/testing/fuzzing/fuzz_support.hh index 7f03d341e1f..6a370f3dfd5 100644 --- a/testing/fuzzing/fuzz_support.hh +++ b/testing/fuzzing/fuzz_support.hh @@ -16,6 +16,7 @@ #include #include "../../toxcore/tox.h" +#include "../../toxcore/tox_private.h" struct Fuzz_Data { static constexpr bool DEBUG = false; diff --git a/testing/fuzzing/protodump.cc b/testing/fuzzing/protodump.cc index 1a3f8c0700d..6b55b54705d 100644 --- a/testing/fuzzing/protodump.cc +++ b/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 address1; tox_self_get_address(tox1, address1.data()); std::array pk1; diff --git a/testing/fuzzing/protodump_reduce.cc b/testing/fuzzing/protodump_reduce.cc index ae9676ca31b..820a3fc7b88 100644 --- a/testing/fuzzing/protodump_reduce.cc +++ b/testing/fuzzing/protodump_reduce.cc @@ -142,9 +142,11 @@ void TestEndToEnd(Fuzz_Data &input) Ptr 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); diff --git a/testing/fuzzing/toxsave_fuzz_test.cc b/testing/fuzzing/toxsave_fuzz_test.cc index 2cb134cbc1f..f48c2bf0131 100644 --- a/testing/fuzzing/toxsave_fuzz_test.cc +++ b/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 diff --git a/toxcore/tox.c b/toxcore/tox.c index 679f1d89fd8..03e34414a8e 100644 --- a/toxcore/tox.c +++ b/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) { diff --git a/toxcore/tox.h b/toxcore/tox.h index 5e1be14a28f..c3b4ad71b57 100644 --- a/toxcore/tox.h +++ b/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); diff --git a/toxcore/tox_api.c b/toxcore/tox_api.c index 26da9b2490e..6d0d9851c7c 100644 --- a/toxcore/tox_api.c +++ b/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; diff --git a/toxcore/tox_events.h b/toxcore/tox_events.h index b69683e9c01..1c9b1b455c2 100644 --- a/toxcore/tox_events.h +++ b/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); diff --git a/toxcore/tox_private.h b/toxcore/tox_private.h index ac6fd4fa771..fe86479a5dd 100644 --- a/toxcore/tox_private.h +++ b/toxcore/tox_private.h @@ -18,21 +18,32 @@ extern "C" { typedef uint64_t tox_mono_time_cb(void *user_data); -struct Tox_System { +typedef struct Tox_System { tox_mono_time_cb *mono_time_callback; void *mono_time_user_data; const struct Random *rng; const struct Network *ns; const struct Memory *mem; -}; +} Tox_System; Tox_System tox_default_system(void); +const Tox_System *tox_get_system(Tox *tox); + +typedef struct Tox_Options_Testing { + const struct Tox_System *operating_system; +} Tox_Options_Testing; + +typedef enum Tox_Err_New_Testing { + TOX_ERR_NEW_TESTING_OK, + TOX_ERR_NEW_TESTING_NULL, +} Tox_Err_New_Testing; + +Tox *tox_new_testing(const Tox_Options *options, Tox_Err_New *error, const Tox_Options_Testing *testing, Tox_Err_New_Testing *testing_error); + void tox_lock(const Tox *tox); void tox_unlock(const Tox *tox); -const Tox_System *tox_get_system(Tox *tox); - /** * Set the callback for the `friend_lossy_packet` event for a specific packet * ID. Pass NULL to unset.