From cdf954afd505537c05fd10b712eed645967a873d Mon Sep 17 00:00:00 2001 From: iphydf Date: Sat, 28 Dec 2024 17:30:48 +0000 Subject: [PATCH] refactor: Make Tox_Options own the passed proxy host and savedata. This way, client code can immediately free their data and can pass temporaries to the options setters. --- auto_tests/file_saving_test.c | 11 +++--- toxcore/tox.h | 39 +++++++++++++++++---- toxcore/tox_api.c | 66 ++++++++++++++++++++++++++++++++--- 3 files changed, 100 insertions(+), 16 deletions(-) diff --git a/auto_tests/file_saving_test.c b/auto_tests/file_saving_test.c index 58a1b6def4..4b12a1503f 100644 --- a/auto_tests/file_saving_test.c +++ b/auto_tests/file_saving_test.c @@ -13,9 +13,9 @@ #include #include -#include "../testing/misc_tools.h" #include "../toxcore/ccompat.h" #include "auto_test_support.h" +#include "c-toxcore/toxcore/tox.h" #include "check_compat.h" #include "../toxencryptsave/toxencryptsave.h" @@ -70,7 +70,8 @@ static void load_data_decrypted(void) uint8_t *cipher = (uint8_t *)malloc(size); ck_assert(cipher != nullptr); - uint8_t *clear = (uint8_t *)malloc(size - TOX_PASS_ENCRYPTION_EXTRA_LENGTH); + const size_t clear_size = size - TOX_PASS_ENCRYPTION_EXTRA_LENGTH; + uint8_t *clear = (uint8_t *)malloc(clear_size); ck_assert(clear != nullptr); size_t read_value = fread(cipher, sizeof(*cipher), size, f); printf("Read read_value = %u of %u\n", (unsigned)read_value, (unsigned)size); @@ -83,9 +84,10 @@ static void load_data_decrypted(void) struct Tox_Options *options = tox_options_new(nullptr); ck_assert(options != nullptr); + tox_options_set_experimental_owned_data(options, true); tox_options_set_savedata_type(options, TOX_SAVEDATA_TYPE_TOX_SAVE); - - tox_options_set_savedata_data(options, clear, size); + tox_options_set_savedata_data(options, clear, clear_size); + free(clear); Tox_Err_New err; @@ -103,7 +105,6 @@ static void load_data_decrypted(void) "name returned by tox_self_get_name does not match expected result"); tox_kill(t); - free(clear); free(cipher); free(readname); fclose(f); diff --git a/toxcore/tox.h b/toxcore/tox.h index 56b5e9ffbd..d45c46c309 100644 --- a/toxcore/tox.h +++ b/toxcore/tox.h @@ -564,9 +564,6 @@ struct Tox_Options { * * This member is ignored (it can be NULL) if proxy_type is * TOX_PROXY_TYPE_NONE. - * - * The data pointed at by this member is owned by the user, so must - * outlive the options object. */ const char *proxy_host; @@ -621,10 +618,7 @@ struct Tox_Options { Tox_Savedata_Type savedata_type; /** - * The savedata. - * - * The data pointed at by this member is owned by the user, so must outlive - * the options object. + * The savedata (either a Tox save or a secret key) to load from. */ const uint8_t *savedata_data; @@ -686,6 +680,33 @@ struct Tox_Options { * Default: false. May become true in the future (0.3.0). */ bool experimental_disable_dns; + + /** + * @brief Whether the savedata data is owned by the Tox_Options object. + * + * If true, the setters for savedata and proxy_host try to copy the string. + * If that fails, the value is not copied and the member is set to the + * user-provided pointer. In that case, the user must not free the string + * until the Tox_Options object is freed. Client code can check whether + * allocation succeeded by comparing the value of the member to the + * user-provided pointer. + * + * If set to true, this must be set before any other member that allocates + * memory is set. + */ + bool experimental_owned_data; + + /** + * @brief Owned pointer to the savedata data. + * @private + */ + uint8_t *owned_savedata_data; + + /** + * @brief Owned pointer to the proxy host. + * @private + */ + char *owned_proxy_host; }; bool tox_options_get_ipv6_enabled(const Tox_Options *options); @@ -752,6 +773,10 @@ void *tox_options_get_log_user_data(const Tox_Options *options); void tox_options_set_log_user_data(Tox_Options *options, void *log_user_data); +bool tox_options_get_experimental_owned_data(const Tox_Options *options); + +void tox_options_set_experimental_owned_data(Tox_Options *options, bool experimental_owned_data); + 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); diff --git a/toxcore/tox_api.c b/toxcore/tox_api.c index b979e4e1e2..8390de92f1 100644 --- a/toxcore/tox_api.c +++ b/toxcore/tox_api.c @@ -3,7 +3,8 @@ */ #include "tox.h" -#include +#include // free, malloc +#include // memcpy, strlen #include "ccompat.h" #include "tox_private.h" @@ -166,7 +167,30 @@ const char *tox_options_get_proxy_host(const Tox_Options *options) } void tox_options_set_proxy_host(Tox_Options *options, const char *proxy_host) { - options->proxy_host = proxy_host; + if (!options->experimental_owned_data) { + options->proxy_host = proxy_host; + return; + } + + if (options->owned_proxy_host != nullptr) { + free(options->owned_proxy_host); + options->owned_proxy_host = nullptr; + } + if (proxy_host == nullptr) { + options->proxy_host = nullptr; + return; + } + + const size_t proxy_host_length = strlen(proxy_host) + 1; + char *owned_ptr = (char *)malloc(proxy_host_length); + if (owned_ptr != nullptr) { + memcpy(owned_ptr, proxy_host, proxy_host_length); + options->proxy_host = owned_ptr; + options->owned_proxy_host = owned_ptr; + } else { + options->proxy_host = proxy_host; + options->owned_proxy_host = nullptr; + } } uint16_t tox_options_get_proxy_port(const Tox_Options *options) { @@ -290,13 +314,42 @@ const uint8_t *tox_options_get_savedata_data(const Tox_Options *options) void tox_options_set_savedata_data(Tox_Options *options, const uint8_t *savedata_data, size_t length) { - options->savedata_data = savedata_data; + if (!options->experimental_owned_data) { + options->savedata_data = savedata_data; + options->savedata_length = length; + return; + } + + if (options->owned_savedata_data != nullptr) { + free(options->owned_savedata_data); + options->owned_savedata_data = nullptr; + } + if (savedata_data == nullptr) { + options->savedata_data = nullptr; + options->savedata_length = 0; + return; + } + + uint8_t *owned_ptr = (uint8_t *)malloc(length); + if (owned_ptr != nullptr) { + memcpy(owned_ptr, savedata_data, length); + options->savedata_data = owned_ptr; + options->owned_savedata_data = owned_ptr; + } else { + options->savedata_data = savedata_data; + options->owned_savedata_data = nullptr; + } options->savedata_length = length; } void tox_options_default(Tox_Options *options) { if (options != nullptr) { + // Free any owned data. + tox_options_set_proxy_host(options, nullptr); + tox_options_set_savedata_data(options, nullptr, 0); + + // Set the rest to default values. const Tox_Options default_options = {false}; *options = default_options; tox_options_set_ipv6_enabled(options, true); @@ -327,7 +380,12 @@ Tox_Options *tox_options_new(Tox_Err_Options_New *error) void tox_options_free(Tox_Options *options) { - free(options); + if (options != nullptr) { + // Free any owned data. + tox_options_set_proxy_host(options, nullptr); + tox_options_set_savedata_data(options, nullptr, 0); + free(options); + } } const char *tox_user_status_to_string(Tox_User_Status value)