Skip to content

Commit

Permalink
refactor: Don't expose Tox_System in the public API
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
nurupo committed Mar 17, 2024
1 parent a3d1b85 commit 0ec4978
Show file tree
Hide file tree
Showing 11 changed files with 82 additions and 47 deletions.
8 changes: 6 additions & 2 deletions testing/fuzzing/bootstrap_fuzz_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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};

Expand Down
8 changes: 6 additions & 2 deletions testing/fuzzing/e2e_fuzz_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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
Expand All @@ -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);

Expand Down
1 change: 1 addition & 0 deletions testing/fuzzing/fuzz_support.hh
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include <vector>

#include "../../toxcore/tox.h"
#include "../../toxcore/tox_private.h"

struct Fuzz_Data {
static constexpr bool DEBUG = false;
Expand Down
12 changes: 8 additions & 4 deletions testing/fuzzing/protodump.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down
8 changes: 6 additions & 2 deletions testing/fuzzing/protodump_reduce.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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
Expand All @@ -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);

Expand Down
9 changes: 5 additions & 4 deletions testing/fuzzing/toxsave_fuzz_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
35 changes: 33 additions & 2 deletions toxcore/tox.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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) {
Expand Down Expand Up @@ -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;

Check warning on line 1033 in toxcore/tox.c

View check run for this annotation

Codecov / codecov/patch

toxcore/tox.c#L1031-L1033

Added lines #L1031 - L1033 were not covered by tests
}

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;

Check warning on line 1039 in toxcore/tox.c

View check run for this annotation

Codecov / codecov/patch

toxcore/tox.c#L1037-L1039

Added lines #L1037 - L1039 were not covered by tests
}

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;

Check warning on line 1047 in toxcore/tox.c

View check run for this annotation

Codecov / codecov/patch

toxcore/tox.c#L1045-L1047

Added lines #L1045 - L1047 were not covered by tests
}

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) {
Expand Down
19 changes: 0 additions & 19 deletions toxcore/tox.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
8 changes: 0 additions & 8 deletions toxcore/tox_api.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions toxcore/tox_events.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
19 changes: 15 additions & 4 deletions toxcore/tox_private.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 0ec4978

Please sign in to comment.