Skip to content

Commit

Permalink
refactor: Make Tox_Options own the passed proxy host and savedata.
Browse files Browse the repository at this point in the history
This way, client code can immediately free their data and can pass
temporaries to the options setters.
  • Loading branch information
iphydf committed Dec 28, 2024
1 parent 8a96816 commit 523340a
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 16 deletions.
11 changes: 6 additions & 5 deletions auto_tests/file_saving_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
#include <stdlib.h>
#include <string.h>

#include "../testing/misc_tools.h"
#include "../toxcore/ccompat.h"
#include "../toxcore/tox.h"
#include "auto_test_support.h"
#include "check_compat.h"

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

Expand All @@ -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);
Expand Down
39 changes: 32 additions & 7 deletions toxcore/tox.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

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

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
66 changes: 62 additions & 4 deletions toxcore/tox_api.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
*/
#include "tox.h"

#include <stdlib.h>
#include <stdlib.h> // free, malloc
#include <string.h> // memcpy, strlen

#include "ccompat.h"
#include "tox_private.h"
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 523340a

Please sign in to comment.