refactor: Add tox_options_set_savedata, deprecate old functions.

iphydf · iphydf · commit 092015c51812 · 2025-01-25T15:01:42.000Z
This one is simpler than the existing tox_options_set_savedata_data and
therefore safer for clients to use. It will either fail (no change) or
succeed (allocation+copy).

Also setting data size without setting data makes no sense, so that
function is now deprecated.
diff --git a/auto_tests/auto_test_support.c b/auto_tests/auto_test_support.c
@@ -3,11 +3,10 @@
 
 #include "check_compat.h"
 #include "../testing/misc_tools.h"
-#include "../toxcore/Messenger.h"
 #include "../toxcore/mono_time.h"
 #include "../toxcore/tox_dispatch.h"
 #include "../toxcore/tox_events.h"
-#include "../toxcore/tox_struct.h"
+#include "../toxcore/tox_struct.h" // IWYU pragma: keep
 
 #include "auto_test_support.h"
 
@@ -211,7 +210,7 @@ void reload(AutoTox *autotox)
     ck_assert(options != nullptr);
     tox_options_set_ipv6_enabled(options, USE_IPV6);
     tox_options_set_savedata_type(options, TOX_SAVEDATA_TYPE_TOX_SAVE);
-    tox_options_set_savedata_data(options, autotox->save_state, autotox->save_size);
+    ck_assert(tox_options_set_savedata(options, autotox->save_state, autotox->save_size));
     autotox->tox = tox_new_log(options, nullptr, &autotox->index);
     ck_assert(autotox->tox != nullptr);
     autotox->dispatch = tox_dispatch_new(nullptr);
diff --git a/auto_tests/conference_test.c b/auto_tests/conference_test.c
@@ -248,7 +248,7 @@ static void run_conference_tests(AutoTox *autotoxes)
             struct Tox_Options *const options = tox_options_new(nullptr);
             ck_assert(options != nullptr);
             tox_options_set_savedata_type(options, TOX_SAVEDATA_TYPE_TOX_SAVE);
-            tox_options_set_savedata_data(options, save[i], save_size[i]);
+            ck_assert(tox_options_set_savedata(options, save[i], save_size[i]));
             autotoxes[i].tox = tox_new_log(options, nullptr, &autotoxes[i].index);
             ck_assert(autotoxes[i].tox != nullptr);
             tox_options_free(options);
diff --git a/auto_tests/encryptsave_test.c b/auto_tests/encryptsave_test.c
@@ -3,7 +3,6 @@
 #include <string.h>
 #include <sys/types.h>
 
-#include "../testing/misc_tools.h"
 #include "../toxcore/ccompat.h"
 #include "../toxcore/crypto_core.h"
 #include "../toxcore/tox.h"
@@ -54,7 +53,7 @@ static void test_save_friend(void)
     struct Tox_Options *options = tox_options_new(nullptr);
     ck_assert(options != nullptr);
     tox_options_set_savedata_type(options, TOX_SAVEDATA_TYPE_TOX_SAVE);
-    tox_options_set_savedata_data(options, enc_data, size2);
+    ck_assert(tox_options_set_savedata(options, enc_data, size2));
 
     Tox_Err_New err2;
     Tox *tox3 = tox_new_log(options, &err2, nullptr);
@@ -66,7 +65,7 @@ static void test_save_friend(void)
     Tox_Err_Decryption err3;
     ret = tox_pass_decrypt(enc_data, size2, (const uint8_t *)"correcthorsebatterystaple", 25, dec_data, &err3);
     ck_assert_msg(ret, "failed to decrypt save: %d", err3);
-    tox_options_set_savedata_data(options, dec_data, size);
+    ck_assert(tox_options_set_savedata(options, dec_data, size));
     tox3 = tox_new_log(options, &err2, nullptr);
     ck_assert_msg(err2 == TOX_ERR_NEW_OK, "failed to load from decrypted data: %d", err2);
     uint8_t address2[TOX_PUBLIC_KEY_SIZE];
@@ -102,7 +101,7 @@ static void test_save_friend(void)
 
     // and now with the code in use (I only bothered with manually to debug this, and it seems a waste
     // to remove the manual check now that it's there)
-    tox_options_set_savedata_data(options, out1, size);
+    ck_assert(tox_options_set_savedata(options, out1, size));
     Tox *tox4 = tox_new_log(options, &err2, nullptr);
     ck_assert_msg(err2 == TOX_ERR_NEW_OK, "failed to new the third");
     uint8_t address5[TOX_PUBLIC_KEY_SIZE];
diff --git a/auto_tests/file_saving_test.c b/auto_tests/file_saving_test.c
@@ -91,9 +91,8 @@ 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);
-    ck_assert(tox_options_set_savedata_data(options, clear, clear_size));
+    ck_assert(tox_options_set_savedata(options, clear, clear_size));
     free(clear);
 
     Tox_Err_New err;
diff --git a/auto_tests/group_save_test.c b/auto_tests/group_save_test.c
@@ -246,7 +246,7 @@ static void group_save_test(AutoTox *autotoxes)
     ck_assert(options != nullptr);
 
     tox_options_set_savedata_type(options, TOX_SAVEDATA_TYPE_TOX_SAVE);
-    tox_options_set_savedata_data(options, save, save_length);
+    ck_assert(tox_options_set_savedata(options, save, save_length));
     tox_options_set_experimental_groups_persistence(options, true);
 
     Tox *new_tox = tox_new_log(options, nullptr, nullptr);
diff --git a/auto_tests/save_compatibility_test.c b/auto_tests/save_compatibility_test.c
@@ -79,7 +79,7 @@ static void test_save_compatibility(const char *save_path)
     ck_assert_msg(save_data != nullptr, "error while reading save file '%s'", save_path);
 
     tox_options_set_savedata_type(options, TOX_SAVEDATA_TYPE_TOX_SAVE);
-    tox_options_set_savedata_data(options, save_data, size);
+    ck_assert(tox_options_set_savedata(options, save_data, size));
 
     size_t index = 0;
     Tox_Err_New err;
diff --git a/auto_tests/save_friend_test.c b/auto_tests/save_friend_test.c
@@ -149,11 +149,12 @@ int main(void)
 
     size_t save_size = tox_get_savedata_size(tox1);
     uint8_t *savedata = (uint8_t *)malloc(save_size);
+    ck_assert(savedata != nullptr);
     tox_get_savedata(tox1, savedata);
 
     struct Tox_Options *const options = tox_options_new(nullptr);
     tox_options_set_savedata_type(options, TOX_SAVEDATA_TYPE_TOX_SAVE);
-    tox_options_set_savedata_data(options, savedata, save_size);
+    ck_assert(tox_options_set_savedata(options, savedata, save_size));
 
     Tox *const tox_to_compare = tox_new_log(options, nullptr, nullptr);
 
diff --git a/auto_tests/save_load_test.c b/auto_tests/save_load_test.c
@@ -9,8 +9,7 @@
 #include "../testing/misc_tools.h"
 #include "../toxcore/ccompat.h"
 #include "../toxcore/tox.h"
-#include "../toxcore/tox_struct.h"
-#include "../toxcore/util.h"
+#include "../toxcore/tox_struct.h" // IWYU pragma: keep
 #include "auto_test_support.h"
 #include "check_compat.h"
 
@@ -88,7 +87,7 @@ static void reload_tox(Tox **tox, struct Tox_Options *const in_opts, void *user_
 
     tox_options_set_savedata_type(options, TOX_SAVEDATA_TYPE_TOX_SAVE);
 
-    tox_options_set_savedata_data(options, buffer + extra, save_size1);
+    ck_assert(tox_options_set_savedata(options, buffer + extra, save_size1));
 
     *tox = tox_new_log(options, nullptr, user_data);
 
diff --git a/other/fun/create_savedata.c b/other/fun/create_savedata.c
@@ -17,7 +17,7 @@
 #include "../../toxcore/tox.h"
 #include "create_common.h"
 
-static bool create_tox(const unsigned char *const secret_key, Tox **const tox)
+static bool create_tox(const uint8_t secret_key[TOX_SECRET_KEY_SIZE], Tox **const tox)
 {
     Tox_Err_Options_New options_error;
     struct Tox_Options *const options = tox_options_new(&options_error);
@@ -28,7 +28,10 @@ static bool create_tox(const unsigned char *const secret_key, Tox **const tox)
     }
 
     tox_options_set_savedata_type(options, TOX_SAVEDATA_TYPE_SECRET_KEY);
-    tox_options_set_savedata_data(options, secret_key, crypto_box_SECRETKEYBYTES);
+    if (!tox_options_set_savedata(options, secret_key, TOX_SECRET_KEY_SIZE)) {
+        tox_options_free(options);
+        return false;
+    }
     Tox_Err_New tox_error;
     *tox = tox_new(options, &tox_error);
 
diff --git a/testing/fuzzing/toxsave_fuzz_test.cc b/testing/fuzzing/toxsave_fuzz_test.cc
@@ -21,7 +21,10 @@ void TestSaveDataLoading(Fuzz_Data &input)
     CONSUME_OR_RETURN(const uint8_t *savedata, input, savedata_size);
 
     // pass test data to Tox
-    tox_options_set_savedata_data(tox_options, savedata, savedata_size);
+    if (!tox_options_set_savedata(tox_options, savedata, savedata_size)) {
+        tox_options_free(tox_options);
+        return;
+    }
     tox_options_set_savedata_type(tox_options, TOX_SAVEDATA_TYPE_TOX_SAVE);
 
     Tox_Options_Testing tox_options_testing;
diff --git a/toxcore/tox_options.c b/toxcore/tox_options.c
@@ -123,14 +123,6 @@ void tox_options_set_savedata_type(Tox_Options *options, Tox_Savedata_Type saved
 {
     options->savedata_type = savedata_type;
 }
-size_t tox_options_get_savedata_length(const Tox_Options *options)
-{
-    return options->savedata_length;
-}
-void tox_options_set_savedata_length(Tox_Options *options, size_t savedata_length)
-{
-    options->savedata_length = savedata_length;
-}
 tox_log_cb *tox_options_get_log_callback(const Tox_Options *options)
 {
     return options->log_callback;
@@ -199,43 +191,71 @@ void tox_options_set_experimental_owned_data(
     options->experimental_owned_data = experimental_owned_data;
 }
 
-const uint8_t *tox_options_get_savedata_data(const Tox_Options *options)
+void tox_options_get_savedata(const Tox_Options *options, uint8_t *savedata)
 {
-    return options->savedata_data;
+    memcpy(savedata, options->savedata_data, options->savedata_length);
 }
 
-bool tox_options_set_savedata_data(Tox_Options *options, const uint8_t *savedata_data, size_t length)
+size_t tox_options_get_savedata_size(const Tox_Options *options)
 {
-    if (!options->experimental_owned_data) {
-        options->savedata_data = savedata_data;
-        options->savedata_length = length;
-        return true;
-    }
+    return options->savedata_length;
+}
 
+bool tox_options_set_savedata(Tox_Options *options, const uint8_t *savedata, size_t savedata_size)
+{
     if (options->owned_savedata_data != nullptr) {
         free(options->owned_savedata_data);
         options->owned_savedata_data = nullptr;
     }
-    if (savedata_data == nullptr) {
+    if (savedata == nullptr) {
         options->savedata_data = nullptr;
         options->savedata_length = 0;
         return true;
     }
 
-    uint8_t *owned_ptr = (uint8_t *)malloc(length);
+    uint8_t *owned_ptr = (uint8_t *)malloc(savedata_size);
     if (owned_ptr == nullptr) {
+        return false;
+    }
+
+    memcpy(owned_ptr, savedata, savedata_size);
+    options->savedata_data = owned_ptr;
+    options->savedata_length = savedata_size;
+    options->owned_savedata_data = owned_ptr;
+    return true;
+}
+
+// Deprecated functions.
+bool tox_options_set_savedata_data(
+    Tox_Options *options, const uint8_t savedata_data[], size_t length)
+{
+    if (!options->experimental_owned_data) {
+        options->savedata_data = savedata_data;
+        options->savedata_length = length;
+        return true;
+    }
+
+    if (!tox_options_set_savedata(options, savedata_data, length)) {
         options->savedata_data = savedata_data;
         options->savedata_length = length;
         options->owned_savedata_data = nullptr;
         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_set_savedata_length(Tox_Options *options, size_t savedata_length)
+{
+    options->savedata_length = savedata_length;
+}
+const uint8_t *tox_options_get_savedata_data(const Tox_Options *options)
+{
+    return options->savedata_data;
+}
+size_t tox_options_get_savedata_length(const Tox_Options *options)
+{
+    return tox_options_get_savedata_size(options);
+}
 
 void tox_options_default(Tox_Options *options)
 {
@@ -279,7 +299,7 @@ void tox_options_free(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);
+        tox_options_set_savedata(options, nullptr, 0);
         free(options);
     }
 }
diff --git a/toxcore/tox_options.h b/toxcore/tox_options.h
diff --git a/toxcore/tox_test.cc b/toxcore/tox_test.cc
