From f7569ed8d478a7022f246b545600d88cee4f9fee Mon Sep 17 00:00:00 2001 From: iphydf Date: Sat, 28 Dec 2024 11:49:03 +0000 Subject: [PATCH] refactor: Use tox memory for group moderation/pack allocations. --- .cirrus.yml | 7 ++--- toxcore/group_moderation.c | 53 +++++++++++++++++----------------- toxcore/group_onion_announce.c | 4 +++ toxcore/group_pack.c | 29 ++++++++++--------- 4 files changed, 50 insertions(+), 43 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index cc309170c8..1efbdf411b 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -8,9 +8,9 @@ bazel-opt_task: configure_script: - git submodule update --init --recursive - /src/workspace/tools/inject-repo c-toxcore + - cat .bazelrc.local test_all_script: - cd /src/workspace && bazel - --max_idle_secs=5 test -k --build_tag_filters=-haskell --test_tag_filters=-haskell @@ -27,9 +27,9 @@ bazel-dbg_task: configure_script: - git submodule update --init --recursive - /src/workspace/tools/inject-repo c-toxcore + - cat .bazelrc.local test_all_script: - cd /src/workspace && bazel - --max_idle_secs=5 test -k --remote_cache=http://$CIRRUS_HTTP_CACHE_HOST --build_tag_filters=-haskell @@ -47,9 +47,9 @@ bazel-msan_task: configure_script: - git submodule update --init --recursive - /src/workspace/tools/inject-repo c-toxcore + - cat .bazelrc.local test_all_script: - cd /src/workspace && bazel - --max_idle_secs=5 test -k --remote_cache=http://$CIRRUS_HTTP_CACHE_HOST -- @@ -66,7 +66,6 @@ cimple_task: - /src/workspace/tools/inject-repo c-toxcore test_all_script: - cd /src/workspace && bazel - --max_idle_secs=5 test -k --build_tag_filters=haskell --test_tag_filters=haskell diff --git a/toxcore/group_moderation.c b/toxcore/group_moderation.c index 3cb69e9403..6c79da44b7 100644 --- a/toxcore/group_moderation.c +++ b/toxcore/group_moderation.c @@ -11,7 +11,7 @@ #include -#include + #include #include @@ -20,6 +20,7 @@ #include "ccompat.h" #include "crypto_core.h" #include "logger.h" +#include "mem.h" #include "network.h" #include "util.h" @@ -51,7 +52,7 @@ int mod_list_unpack(Moderation *moderation, const uint8_t *data, uint16_t length return 0; } - uint8_t **tmp_list = (uint8_t **)calloc(num_mods, sizeof(uint8_t *)); + uint8_t **tmp_list = (uint8_t **)mem_valloc(moderation->mem, num_mods, sizeof(uint8_t *)); if (tmp_list == nullptr) { return -1; @@ -60,7 +61,7 @@ int mod_list_unpack(Moderation *moderation, const uint8_t *data, uint16_t length uint16_t unpacked_len = 0; for (uint16_t i = 0; i < num_mods; ++i) { - uint8_t *entry = (uint8_t *)malloc(MOD_LIST_ENTRY_SIZE); + uint8_t *entry = (uint8_t *)mem_balloc(moderation->mem, MOD_LIST_ENTRY_SIZE); if (entry == nullptr) { free_uint8_t_pointer_array(moderation->mem, tmp_list, i); @@ -102,7 +103,7 @@ bool mod_list_make_hash(const Moderation *moderation, uint8_t *hash) assert(data_buf_size > 0); - uint8_t *data = (uint8_t *)malloc(data_buf_size); + uint8_t *data = (uint8_t *)mem_balloc(moderation->mem, data_buf_size); if (data == nullptr) { return false; @@ -112,7 +113,7 @@ bool mod_list_make_hash(const Moderation *moderation, uint8_t *hash) mod_list_get_data_hash(hash, data, data_buf_size); - free(data); + mem_delete(moderation->mem, data); return true; } @@ -166,10 +167,10 @@ bool mod_list_remove_index(Moderation *moderation, uint16_t index) MOD_LIST_ENTRY_SIZE); } - free(moderation->mod_list[moderation->num_mods]); + mem_delete(moderation->mem, moderation->mod_list[moderation->num_mods]); moderation->mod_list[moderation->num_mods] = nullptr; - uint8_t **tmp_list = (uint8_t **)realloc(moderation->mod_list, moderation->num_mods * sizeof(uint8_t *)); + uint8_t **tmp_list = (uint8_t **)mem_vrealloc(moderation->mem, moderation->mod_list, moderation->num_mods, sizeof(uint8_t *)); if (tmp_list == nullptr) { return false; @@ -203,7 +204,7 @@ bool mod_list_add_entry(Moderation *moderation, const uint8_t *mod_data) return false; } - uint8_t **tmp_list = (uint8_t **)realloc(moderation->mod_list, (moderation->num_mods + 1) * sizeof(uint8_t *)); + uint8_t **tmp_list = (uint8_t **)mem_vrealloc(moderation->mem, moderation->mod_list, moderation->num_mods + 1, sizeof(uint8_t *)); if (tmp_list == nullptr) { return false; @@ -211,7 +212,7 @@ bool mod_list_add_entry(Moderation *moderation, const uint8_t *mod_data) moderation->mod_list = tmp_list; - uint8_t *entry = (uint8_t *)malloc(MOD_LIST_ENTRY_SIZE); + uint8_t *entry = (uint8_t *)mem_balloc(moderation->mem, MOD_LIST_ENTRY_SIZE); if (entry == nullptr) { return false; @@ -405,8 +406,8 @@ int sanctions_list_unpack(Mod_Sanction *sanctions, Mod_Sanction_Creds *creds, ui * * Return true on success. */ -non_null(4) nullable(1) -static bool sanctions_list_make_hash(const Mod_Sanction *sanctions, uint32_t new_version, uint16_t num_sanctions, +non_null(1, 5) nullable(2) +static bool sanctions_list_make_hash(const Memory *mem, const Mod_Sanction *sanctions, uint32_t new_version, uint16_t num_sanctions, uint8_t *hash) { if (num_sanctions == 0 || sanctions == nullptr) { @@ -422,7 +423,7 @@ static bool sanctions_list_make_hash(const Mod_Sanction *sanctions, uint32_t new return false; } - uint8_t *data = (uint8_t *)malloc(data_buf_size); + uint8_t *data = (uint8_t *)mem_balloc(mem, data_buf_size); if (data == nullptr) { return false; @@ -435,7 +436,7 @@ static bool sanctions_list_make_hash(const Mod_Sanction *sanctions, uint32_t new memcpy(&data[sig_data_size], &new_version, sizeof(uint32_t)); crypto_sha256(hash, data, data_buf_size); - free(data); + mem_delete(mem, data); return true; } @@ -492,7 +493,7 @@ bool sanctions_list_make_creds(Moderation *moderation) uint8_t hash[MOD_SANCTION_HASH_SIZE]; - if (!sanctions_list_make_hash(moderation->sanctions, moderation->sanctions_creds.version, + if (!sanctions_list_make_hash(moderation->mem, moderation->sanctions, moderation->sanctions_creds.version, moderation->num_sanctions, hash)) { moderation->sanctions_creds = old_creds; return false; @@ -533,7 +534,7 @@ static bool sanctions_creds_validate(const Moderation *moderation, const Mod_San uint8_t hash[MOD_SANCTION_HASH_SIZE]; - if (!sanctions_list_make_hash(sanctions, creds->version, num_sanctions, hash)) { + if (!sanctions_list_make_hash(moderation->mem, sanctions, creds->version, num_sanctions, hash)) { return false; } @@ -612,9 +613,9 @@ static bool sanctions_apply_new(Moderation *moderation, Mod_Sanction *new_sancti * memory returned by this function. */ non_null() -static Mod_Sanction *sanctions_list_copy(const Mod_Sanction *sanctions, uint16_t num_sanctions) +static Mod_Sanction *sanctions_list_copy(const Memory *mem, const Mod_Sanction *sanctions, uint16_t num_sanctions) { - Mod_Sanction *copy = (Mod_Sanction *)calloc(num_sanctions, sizeof(Mod_Sanction)); + Mod_Sanction *copy = (Mod_Sanction *)mem_valloc(mem, num_sanctions, sizeof(Mod_Sanction)); if (copy == nullptr) { return nullptr; @@ -655,7 +656,7 @@ static bool sanctions_list_remove_index(Moderation *moderation, uint16_t index, } /* Operate on a copy of the list in case something goes wrong. */ - Mod_Sanction *sanctions_copy = sanctions_list_copy(moderation->sanctions, moderation->num_sanctions); + Mod_Sanction *sanctions_copy = sanctions_list_copy(moderation->mem, moderation->sanctions, moderation->num_sanctions); if (sanctions_copy == nullptr) { return false; @@ -665,15 +666,15 @@ static bool sanctions_list_remove_index(Moderation *moderation, uint16_t index, sanctions_copy[index] = sanctions_copy[new_num]; } - Mod_Sanction *new_list = (Mod_Sanction *)realloc(sanctions_copy, new_num * sizeof(Mod_Sanction)); + Mod_Sanction *new_list = (Mod_Sanction *)mem_vrealloc(moderation->mem, sanctions_copy, new_num, sizeof(Mod_Sanction)); if (new_list == nullptr) { - free(sanctions_copy); + mem_delete(moderation->mem, sanctions_copy); return false; } if (!sanctions_apply_new(moderation, new_list, creds, new_num)) { - free(new_list); + mem_delete(moderation->mem, new_list); return false; } @@ -753,7 +754,7 @@ bool sanctions_list_add_entry(Moderation *moderation, const Mod_Sanction *sancti Mod_Sanction *sanctions_copy = nullptr; if (moderation->num_sanctions > 0) { - sanctions_copy = sanctions_list_copy(moderation->sanctions, moderation->num_sanctions); + sanctions_copy = sanctions_list_copy(moderation->mem, moderation->sanctions, moderation->num_sanctions); if (sanctions_copy == nullptr) { return false; @@ -761,17 +762,17 @@ bool sanctions_list_add_entry(Moderation *moderation, const Mod_Sanction *sancti } const uint16_t index = moderation->num_sanctions; - Mod_Sanction *new_list = (Mod_Sanction *)realloc(sanctions_copy, (index + 1) * sizeof(Mod_Sanction)); + Mod_Sanction *new_list = (Mod_Sanction *)mem_vrealloc(moderation->mem, sanctions_copy, index + 1, sizeof(Mod_Sanction)); if (new_list == nullptr) { - free(sanctions_copy); + mem_delete(moderation->mem, sanctions_copy); return false; } new_list[index] = *sanction; if (!sanctions_apply_new(moderation, new_list, creds, index + 1)) { - free(new_list); + mem_delete(moderation->mem, new_list); return false; } @@ -864,7 +865,7 @@ uint16_t sanctions_list_replace_sig(Moderation *moderation, const uint8_t *publi void sanctions_list_cleanup(Moderation *moderation) { - free(moderation->sanctions); + mem_delete(moderation->mem, moderation->sanctions); moderation->sanctions = nullptr; moderation->num_sanctions = 0; diff --git a/toxcore/group_onion_announce.c b/toxcore/group_onion_announce.c index 6d680137b4..e079d79a66 100644 --- a/toxcore/group_onion_announce.c +++ b/toxcore/group_onion_announce.c @@ -20,6 +20,10 @@ #include "onion_announce.h" #include "timed_auth.h" +static_assert(TIMED_AUTH_SIZE <= ONION_ANNOUNCE_REQUEST_MAX_SIZE, + "Timed auth does not fit into the onion packet"); +static_assert(PACKED_NODE_SIZE_IP6 <= GCA_ANNOUNCE_MAX_SIZE, + "IP6 does not fit into the GC_Announce"); static_assert(GCA_ANNOUNCE_MAX_SIZE <= ONION_MAX_EXTRA_DATA_SIZE, "GC_Announce does not fit into the onion packet extra data"); diff --git a/toxcore/group_pack.c b/toxcore/group_pack.c index c20d6a1213..877df7409e 100644 --- a/toxcore/group_pack.c +++ b/toxcore/group_pack.c @@ -10,7 +10,6 @@ #include "group_pack.h" #include -#include #include #include "DHT.h" @@ -23,9 +22,13 @@ #include "group_common.h" #include "group_moderation.h" #include "logger.h" +#include "mem.h" #include "network.h" #include "util.h" +static_assert(GC_SAVED_PEER_SIZE >= sizeof(IP_Port), + "GC_SAVED_PEER_SIZE cannot contain IP_Port"); + bool group_privacy_state_from_int(uint8_t value, Group_Privacy_State *out_enum) { switch (value) { @@ -180,7 +183,7 @@ static bool load_unpack_mod_list(GC_Chat *chat, Bin_Unpack *bu) chat->moderation.num_mods = MOD_MAX_NUM_MODERATORS; } - uint8_t *packed_mod_list = (uint8_t *)malloc(chat->moderation.num_mods * MOD_LIST_ENTRY_SIZE); + uint8_t *packed_mod_list = (uint8_t *)mem_balloc(chat->mem, chat->moderation.num_mods * MOD_LIST_ENTRY_SIZE); if (packed_mod_list == nullptr) { LOGGER_ERROR(chat->log, "Failed to allocate memory for packed mod list"); @@ -191,17 +194,17 @@ static bool load_unpack_mod_list(GC_Chat *chat, Bin_Unpack *bu) if (!bin_unpack_bin_fixed(bu, packed_mod_list, packed_size)) { LOGGER_ERROR(chat->log, "Failed to unpack mod list binary data"); - free(packed_mod_list); + mem_delete(chat->mem, packed_mod_list); return false; } if (mod_list_unpack(&chat->moderation, packed_mod_list, packed_size, chat->moderation.num_mods) == -1) { LOGGER_ERROR(chat->log, "Failed to unpack mod list info"); - free(packed_mod_list); + mem_delete(chat->mem, packed_mod_list); return false; } - free(packed_mod_list); + mem_delete(chat->mem, packed_mod_list); return true; } @@ -299,7 +302,7 @@ static bool load_unpack_saved_peers(GC_Chat *chat, Bin_Unpack *bu) return true; } - uint8_t *saved_peers = (uint8_t *)malloc(saved_peers_size * GC_SAVED_PEER_SIZE); + uint8_t *saved_peers = (uint8_t *)mem_balloc(chat->mem, saved_peers_size * GC_SAVED_PEER_SIZE); if (saved_peers == nullptr) { LOGGER_ERROR(chat->log, "Failed to allocate memory for saved peer list"); @@ -308,7 +311,7 @@ static bool load_unpack_saved_peers(GC_Chat *chat, Bin_Unpack *bu) if (!bin_unpack_bin_fixed(bu, saved_peers, saved_peers_size)) { LOGGER_ERROR(chat->log, "Failed to unpack saved peers binary data"); - free(saved_peers); + mem_delete(chat->mem, saved_peers); return false; } @@ -316,7 +319,7 @@ static bool load_unpack_saved_peers(GC_Chat *chat, Bin_Unpack *bu) LOGGER_ERROR(chat->log, "Failed to unpack saved peers"); // recoverable error } - free(saved_peers); + mem_delete(chat->mem, saved_peers); return true; } @@ -390,7 +393,7 @@ static void save_pack_mod_list(const GC_Chat *chat, Bin_Pack *bp) return; } - uint8_t *packed_mod_list = (uint8_t *)malloc(num_mods * MOD_LIST_ENTRY_SIZE); + uint8_t *packed_mod_list = (uint8_t *)mem_balloc(chat->mem, num_mods * MOD_LIST_ENTRY_SIZE); // we can still recover without the mod list if (packed_mod_list == nullptr) { @@ -408,7 +411,7 @@ static void save_pack_mod_list(const GC_Chat *chat, Bin_Pack *bp) bin_pack_bin(bp, packed_mod_list, packed_size); // 2 - free(packed_mod_list); + mem_delete(chat->mem, packed_mod_list); } non_null() @@ -445,7 +448,7 @@ static void save_pack_saved_peers(const GC_Chat *chat, Bin_Pack *bp) { bin_pack_array(bp, 2); - uint8_t *saved_peers = (uint8_t *)malloc(GC_MAX_SAVED_PEERS * GC_SAVED_PEER_SIZE); + uint8_t *saved_peers = (uint8_t *)mem_balloc(chat->mem, GC_MAX_SAVED_PEERS * GC_SAVED_PEER_SIZE); // we can still recover without the saved peers list if (saved_peers == nullptr) { @@ -466,13 +469,13 @@ static void save_pack_saved_peers(const GC_Chat *chat, Bin_Pack *bp) if (packed_size == 0) { bin_pack_nil(bp); // 2 - free(saved_peers); + mem_delete(chat->mem, saved_peers); return; } bin_pack_bin(bp, saved_peers, packed_size); // 2 - free(saved_peers); + mem_delete(chat->mem, saved_peers); } void gc_save_pack_group(const GC_Chat *chat, Bin_Pack *bp)