Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Make Tox_Options own the passed proxy host and savedata. #2819

Merged
merged 1 commit into from
Jan 22, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions auto_tests/file_saving_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
#include <stdlib.h>
#include <string.h>

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

Expand Down Expand Up @@ -71,11 +73,12 @@ static void load_data_decrypted(void)
int64_t size = ftell(f);
fseek(f, 0, SEEK_SET);

ck_assert_msg(0 <= size && size <= UINT_MAX, "file size out of range");
ck_assert_msg(TOX_PASS_ENCRYPTION_EXTRA_LENGTH <= size && size <= UINT_MAX, "file size out of range");

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;
Green-Sky marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -88,9 +91,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);
ck_assert(tox_options_set_savedata_data(options, clear, clear_size));
free(clear);

Tox_Err_New err;

Expand All @@ -107,7 +111,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
44 changes: 38 additions & 6 deletions toxcore/tox.h
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ struct Tox_Options {
* TOX_PROXY_TYPE_NONE.
*
* The data pointed at by this member is owned by the user, so must
* outlive the options object.
Green-Sky marked this conversation as resolved.
Show resolved Hide resolved
* outlive the options object (unless experimental_owned_data is set).
*/
const char *proxy_host;

Expand Down Expand Up @@ -629,10 +629,10 @@ struct Tox_Options {
Tox_Savedata_Type savedata_type;

/**
* The savedata.
* The savedata (either a Tox save or a secret key) to load from.
*
* The data pointed at by this member is owned by the user, so must outlive
* the options object.
* The data pointed at by this member is owned by the user, so must
* outlive the options object (unless experimental_owned_data is set).
*/
const uint8_t *savedata_data;

Expand Down Expand Up @@ -694,6 +694,34 @@ 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 checking the returned bool. If
* experimental_owned_data is false, it will always return true. If set to
* true, the return value will be false on allocation failure.
*
* 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;
};
#endif /* TOX_HIDE_DEPRECATED */

Expand All @@ -719,7 +747,7 @@ void tox_options_set_proxy_type(Tox_Options *options, Tox_Proxy_Type proxy_type)

const char *tox_options_get_proxy_host(const Tox_Options *options);

void tox_options_set_proxy_host(Tox_Options *options, const char *proxy_host);
bool tox_options_set_proxy_host(Tox_Options *options, const char *proxy_host);

uint16_t tox_options_get_proxy_port(const Tox_Options *options);

Expand Down Expand Up @@ -747,7 +775,7 @@ void tox_options_set_savedata_type(Tox_Options *options, Tox_Savedata_Type saved

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);
bool tox_options_set_savedata_data(Tox_Options *options, const uint8_t savedata_data[], size_t length);

size_t tox_options_get_savedata_length(const Tox_Options *options);

Expand All @@ -761,6 +789,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
85 changes: 79 additions & 6 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 @@ -164,9 +165,34 @@ const char *tox_options_get_proxy_host(const Tox_Options *options)
{
return options->proxy_host;
}
void tox_options_set_proxy_host(Tox_Options *options, const char *proxy_host)
bool 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 true;
}

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 true;
}

const size_t proxy_host_length = strlen(proxy_host) + 1;
char *owned_ptr = (char *)malloc(proxy_host_length);
if (owned_ptr == nullptr) {
options->proxy_host = proxy_host;
options->owned_proxy_host = nullptr;
robinlinden marked this conversation as resolved.
Show resolved Hide resolved
return false;
}

memcpy(owned_ptr, proxy_host, proxy_host_length);
options->proxy_host = owned_ptr;
options->owned_proxy_host = owned_ptr;
return true;
}
uint16_t tox_options_get_proxy_port(const Tox_Options *options)
{
Expand Down Expand Up @@ -282,21 +308,62 @@ void tox_options_set_experimental_disable_dns(Tox_Options *options, bool experim
{
options->experimental_disable_dns = experimental_disable_dns;
}
bool tox_options_get_experimental_owned_data(const Tox_Options *options)
{
return options->experimental_owned_data;
}
void tox_options_set_experimental_owned_data(
Tox_Options *options, bool experimental_owned_data)
{
options->experimental_owned_data = experimental_owned_data;
}

const uint8_t *tox_options_get_savedata_data(const Tox_Options *options)
{
return options->savedata_data;
}

void tox_options_set_savedata_data(Tox_Options *options, const uint8_t *savedata_data, size_t length)
bool 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 true;
}

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 true;
}

uint8_t *owned_ptr = (uint8_t *)malloc(length);
if (owned_ptr == nullptr) {
options->savedata_data = savedata_data;
options->savedata_length = length;
options->owned_savedata_data = nullptr;
Green-Sky marked this conversation as resolved.
Show resolved Hide resolved
return false;
}

memcpy(owned_ptr, savedata_data, length);
options->savedata_data = owned_ptr;
options->savedata_length = length;
options->owned_savedata_data = owned_ptr;
return true;
}

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 All @@ -308,6 +375,7 @@ void tox_options_default(Tox_Options *options)
tox_options_set_experimental_thread_safety(options, false);
tox_options_set_experimental_groups_persistence(options, false);
tox_options_set_experimental_disable_dns(options, false);
tox_options_set_experimental_owned_data(options, false);
}
}

Expand All @@ -327,7 +395,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
Loading