cleanup: Remove bin_pack_{new,free}.

We should only ever use `bin_pack_obj` and friends, which stack-allocate
the packer and pass it to callbacks.

other/bootstrap_daemon/docker/tox-bootstrapd.sha256

@@ -1 +1 @@
-738c98673260593fc150b8d5b0cb770cd521f469b4eb04c873f19d89bb7238cf  /usr/local/bin/tox-bootstrapd
+189633f3accb67886c402bf242616d9b3e8258f8050dbd00a10b7c6147ed28aa  /usr/local/bin/tox-bootstrapd

toxcore/DHT.c

@@ -552,8 +552,8 @@ static bool bin_pack_node_handler(Bin_Pack *bp, const Logger *logger, const void
 
 int pack_nodes(const Logger *logger, uint8_t *data, uint16_t length, const Node_format *nodes, uint16_t number)
 {
-    const uint32_t size = bin_pack_obj_array_size(bin_pack_node_handler, logger, nodes, number);
+    const uint32_t size = bin_pack_obj_array_b_size(bin_pack_node_handler, logger, nodes, number);
-    if (!bin_pack_obj_array(bin_pack_node_handler, logger, nodes, number, data, length)) {
+    if (!bin_pack_obj_array_b(bin_pack_node_handler, logger, nodes, number, data, length)) {
         return -1;
     }
     return size;

toxcore/Messenger.c

@@ -3237,22 +3237,17 @@ static uint8_t *groups_save(const Messenger *m, uint8_t *data)
 }
 
 non_null()
-static State_Load_Status groups_load(Messenger *m, const uint8_t *data, uint32_t length)
+static bool handle_groups_load(Bin_Unpack *bu, void *obj)
 {
-    Bin_Unpack *bu = bin_unpack_new(data, length);
+    Messenger *m = (Messenger *)obj;
-    if (bu == nullptr) {
-        LOGGER_ERROR(m->log, "failed to allocate binary unpacker");
-        return STATE_LOAD_STATUS_ERROR;
-    }
 
     uint32_t num_groups;
     if (!bin_unpack_array(bu, &num_groups)) {
         LOGGER_ERROR(m->log, "msgpack failed to unpack groupchats array: expected array");
-        bin_unpack_free(bu);
+        return false;
-        return STATE_LOAD_STATUS_ERROR;
     }
 
-    LOGGER_DEBUG(m->log, "Loading %u groups (length %u)", num_groups, length);
+    LOGGER_DEBUG(m->log, "Loading %u groups", num_groups);
 
     for (uint32_t i = 0; i < num_groups; ++i) {
         const int group_number = gc_group_load(m->group_handler, bu);
@@ -3266,7 +3261,16 @@ static State_Load_Status groups_load(Messenger *m, const uint8_t *data, uint32_t
 
     LOGGER_DEBUG(m->log, "Successfully loaded %u groups", gc_count_groups(m->group_handler));
 
-    bin_unpack_free(bu);
+    return true;
+}
+
+non_null()
+static State_Load_Status groups_load(Messenger *m, const uint8_t *data, uint32_t length)
+{
+    if (!bin_unpack_obj(handle_groups_load, m, data, length)) {
+        LOGGER_ERROR(m->log, "msgpack failed to unpack groupchats array");
+        return STATE_LOAD_STATUS_ERROR;
+    }
 
     return STATE_LOAD_STATUS_CONTINUE;
 }

toxcore/bin_pack.c

@@ -5,7 +5,6 @@
 #include "bin_pack.h"
 
 #include <assert.h>
-#include <stdlib.h>
 #include <string.h>
 
 #include "../third_party/cmp/cmp.h"
@@ -34,11 +33,11 @@ static bool null_skipper(cmp_ctx_t *ctx, size_t limit)
 }
 
 non_null()
-static size_t buf_writer(cmp_ctx_t *ctx, const void *data, size_t count)
+static size_t buf_writer(cmp_ctx_t *ctx, const void *data, size_t data_size)
 {
     Bin_Pack *bp = (Bin_Pack *)ctx->buf;
     assert(bp != nullptr);
-    const uint32_t new_pos = bp->bytes_pos + count;
+    const uint32_t new_pos = bp->bytes_pos + data_size;
     if (new_pos < bp->bytes_pos) {
         // 32 bit overflow.
         return 0;
@@ -48,10 +47,10 @@ static size_t buf_writer(cmp_ctx_t *ctx, const void *data, size_t count)
             // Buffer too small.
             return 0;
         }
-        memcpy(bp->bytes + bp->bytes_pos, data, count);
+        memcpy(&bp->bytes[bp->bytes_pos], data, data_size);
     }
-    bp->bytes_pos += count;
+    bp->bytes_pos += data_size;
-    return count;
+    return data_size;
 }
 
 non_null(1) nullable(2)
@@ -80,11 +79,11 @@ bool bin_pack_obj(bin_pack_cb *callback, const Logger *logger, const void *obj,
     return callback(&bp, logger, obj);
 }
 
-uint32_t bin_pack_obj_array_size(bin_pack_array_cb *callback, const Logger *logger, const void *arr, uint32_t count)
+uint32_t bin_pack_obj_array_b_size(bin_pack_array_cb *callback, const Logger *logger, const void *arr, uint32_t arr_size)
 {
     Bin_Pack bp;
     bin_pack_init(&bp, nullptr, 0);
-    for (uint32_t i = 0; i < count; ++i) {
+    for (uint32_t i = 0; i < arr_size; ++i) {
         if (!callback(&bp, logger, arr, i)) {
             return UINT32_MAX;
         }
@@ -92,11 +91,11 @@ uint32_t bin_pack_obj_array_size(bin_pack_array_cb *callback, const Logger *logg
     return bp.bytes_pos;
 }
 
-bool bin_pack_obj_array(bin_pack_array_cb *callback, const Logger *logger, const void *arr, uint32_t count, uint8_t *buf, uint32_t buf_size)
+bool bin_pack_obj_array_b(bin_pack_array_cb *callback, const Logger *logger, const void *arr, uint32_t arr_size, uint8_t *buf, uint32_t buf_size)
 {
     Bin_Pack bp;
     bin_pack_init(&bp, buf, buf_size);
-    for (uint32_t i = 0; i < count; ++i) {
+    for (uint32_t i = 0; i < arr_size; ++i) {
         if (!callback(&bp, logger, arr, i)) {
             return false;
         }
@@ -104,21 +103,6 @@ bool bin_pack_obj_array(bin_pack_array_cb *callback, const Logger *logger, const
     return true;
 }
 
-Bin_Pack *bin_pack_new(uint8_t *buf, uint32_t buf_size)
-{
-    Bin_Pack *bp = (Bin_Pack *)calloc(1, sizeof(Bin_Pack));
-    if (bp == nullptr) {
-        return nullptr;
-    }
-    bin_pack_init(bp, buf, buf_size);
-    return bp;
-}
-
-void bin_pack_free(Bin_Pack *bp)
-{
-    free(bp);
-}
-
 bool bin_pack_array(Bin_Pack *bp, uint32_t size)
 {
     return cmp_write_array(&bp->ctx, size);

toxcore/bin_pack.h

@@ -69,27 +69,24 @@ bool bin_pack_obj(bin_pack_cb *callback, const Logger *logger, const void *obj,
 
 /** @brief Determine the serialised size of an object array.
  *
- * Calls the callback `count` times with increasing `index` argument from 0 to
+ * Behaves exactly like `bin_pack_obj_b_array` but doesn't write.
- * `count`. This function is here just so we don't need to write the same
- * trivial loop many times and so we don't need an extra struct just to contain
- * an array with size so it can be passed to `bin_pack_obj_size`.
  *
  * @param callback The function called on the created packer and each object to
  *   be packed.
  * @param logger Optional logger object to pass to the callback.
  * @param arr The object array to be packed, passed as `arr` to the callback.
- * @param count The number of elements in the object array.
+ * @param arr_size The number of elements in the object array.
  *
  * @return The packed size of the passed object array according to the callback.
  * @retval UINT32_MAX in case of errors such as buffer overflow.
  */
 non_null(1, 3) nullable(2)
-uint32_t bin_pack_obj_array_size(bin_pack_array_cb *callback, const Logger *logger, const void *arr, uint32_t count);
+uint32_t bin_pack_obj_array_b_size(bin_pack_array_cb *callback, const Logger *logger, const void *arr, uint32_t arr_size);
 
 /** @brief Pack an object array into a buffer of a given size.
  *
- * Calls the callback `count` times with increasing `index` argument from 0 to
+ * Calls the callback `arr_size` times with increasing `index` argument from 0 to
- * `count`. This function is here just so we don't need to write the same
+ * `arr_size`. This function is here just so we don't need to write the same
  * trivial loop many times and so we don't need an extra struct just to contain
  * an array with size so it can be passed to `bin_pack_obj`.
  *
@@ -100,33 +97,14 @@ uint32_t bin_pack_obj_array_size(bin_pack_array_cb *callback, const Logger *logg
  *   array.
  * @param logger Optional logger object to pass to the callback.
  * @param arr The object array to be packed, passed as `arr` to the callback.
- * @param count The number of elements in the object array.
+ * @param arr_size The number of elements in the object array.
  * @param buf A byte array large enough to hold the serialised representation of `arr`.
  * @param buf_size The size of the byte array. Can be `UINT32_MAX` to disable bounds checking.
  *
  * @retval false if an error occurred (e.g. buffer overflow).
  */
 non_null(1, 3, 5) nullable(2)
-bool bin_pack_obj_array(bin_pack_array_cb *callback, const Logger *logger, const void *arr, uint32_t count, uint8_t *buf, uint32_t buf_size);
+bool bin_pack_obj_array_b(bin_pack_array_cb *callback, const Logger *logger, const void *arr, uint32_t arr_size, uint8_t *buf, uint32_t buf_size);
-
-/** @brief Allocate a new packer object.
- *
- * This is the only function that allocates memory in this module.
- *
- * @param buf A byte array large enough to hold the serialised representation of `obj`.
- * @param buf_size The size of the byte array. Can be `UINT32_MAX` to disable bounds checking.
- *
- * @retval nullptr on allocation failure.
- */
-non_null()
-Bin_Pack *bin_pack_new(uint8_t *buf, uint32_t buf_size);
-
-/** @brief Deallocates a packer object.
- *
- * Does not deallocate the buffer inside.
- */
-nullable(1)
-void bin_pack_free(Bin_Pack *bp);
 
 /** @brief Start packing a MessagePack array.
  *

toxcore/bin_pack_test.cc

@@ -10,115 +10,138 @@
 
 namespace {
 
-struct Bin_Pack_Deleter {
-    void operator()(Bin_Pack *bp) const { bin_pack_free(bp); }
-};
-
-using Bin_Pack_Ptr = std::unique_ptr<Bin_Pack, Bin_Pack_Deleter>;
-
-struct Bin_Unpack_Deleter {
-    void operator()(Bin_Unpack *bu) const { bin_unpack_free(bu); }
-};
-
-using Bin_Unpack_Ptr = std::unique_ptr<Bin_Unpack, Bin_Unpack_Deleter>;
-
 TEST(BinPack, TooSmallBufferIsNotExceeded)
 {
-    std::array<uint8_t, 7> buf;
+    const uint64_t orig = 1234567812345678LL;
-    Bin_Pack_Ptr bp(bin_pack_new(buf.data(), buf.size()));
+    std::array<uint8_t, sizeof(orig) - 1> buf;
-    ASSERT_NE(bp, nullptr);
+    EXPECT_FALSE(bin_pack_obj(
-    EXPECT_FALSE(bin_pack_u64_b(bp.get(), 1234567812345678LL));
+        [](Bin_Pack *bp, const Logger *logger, const void *obj) {
+            return bin_pack_u64_b(bp, *static_cast<const uint64_t *>(obj));
+        },
+        nullptr, &orig, buf.data(), buf.size()));
 }
 
 TEST(BinPack, PackedUint64CanBeUnpacked)
 {
+    const uint64_t orig = 1234567812345678LL;
     std::array<uint8_t, 8> buf;
-    Bin_Pack_Ptr bp(bin_pack_new(buf.data(), buf.size()));
+    EXPECT_TRUE(bin_pack_obj(
-    ASSERT_NE(bp, nullptr);
+        [](Bin_Pack *bp, const Logger *logger, const void *obj) {
-    ASSERT_TRUE(bin_pack_u64_b(bp.get(), 1234567812345678LL));
+            return bin_pack_u64_b(bp, *static_cast<const uint64_t *>(obj));
+        },
+        nullptr, &orig, buf.data(), buf.size()));
 
-    Bin_Unpack_Ptr bu(bin_unpack_new(buf.data(), buf.size()));
+    uint64_t unpacked;
-    ASSERT_NE(bu, nullptr);
+    EXPECT_TRUE(bin_unpack_obj(
-    uint64_t val;
+        [](Bin_Unpack *bu, void *obj) {
-    ASSERT_TRUE(bin_unpack_u64_b(bu.get(), &val));
+            return bin_unpack_u64_b(bu, static_cast<uint64_t *>(obj));
-    EXPECT_EQ(val, 1234567812345678LL);
+        },
+        &unpacked, buf.data(), buf.size()));
+    EXPECT_EQ(unpacked, 1234567812345678LL);
 }
 
 TEST(BinPack, MsgPackedUint8CanBeUnpackedAsUint32)
 {
+    const uint8_t orig = 123;
     std::array<uint8_t, 2> buf;
-    Bin_Pack_Ptr bp(bin_pack_new(buf.data(), buf.size()));
+    EXPECT_TRUE(bin_pack_obj(
-    ASSERT_NE(bp, nullptr);
+        [](Bin_Pack *bp, const Logger *logger, const void *obj) {
-    ASSERT_TRUE(bin_pack_u08(bp.get(), 123));
+            return bin_pack_u08(bp, *static_cast<const uint8_t *>(obj));
+        },
+        nullptr, &orig, buf.data(), buf.size()));
 
-    Bin_Unpack_Ptr bu(bin_unpack_new(buf.data(), buf.size()));
+    uint32_t unpacked;
-    ASSERT_NE(bu, nullptr);
+    EXPECT_TRUE(bin_unpack_obj(
-    uint32_t val;
+        [](Bin_Unpack *bu, void *obj) { return bin_unpack_u32(bu, static_cast<uint32_t *>(obj)); },
-    ASSERT_TRUE(bin_unpack_u32(bu.get(), &val));
+        &unpacked, buf.data(), buf.size()));
-    EXPECT_EQ(val, 123);
+    EXPECT_EQ(unpacked, 123);
 }
 
 TEST(BinPack, MsgPackedUint32CanBeUnpackedAsUint8IfSmallEnough)
 {
+    const uint32_t orig = 123;
     std::array<uint8_t, 2> buf;
-    Bin_Pack_Ptr bp(bin_pack_new(buf.data(), buf.size()));
+    EXPECT_TRUE(bin_pack_obj(
-    ASSERT_NE(bp, nullptr);
+        [](Bin_Pack *bp, const Logger *logger, const void *obj) {
-    ASSERT_TRUE(bin_pack_u32(bp.get(), 123));
+            return bin_pack_u32(bp, *static_cast<const uint32_t *>(obj));
+        },
+        nullptr, &orig, buf.data(), buf.size()));
 
-    Bin_Unpack_Ptr bu(bin_unpack_new(buf.data(), buf.size()));
+    uint8_t unpacked;
-    ASSERT_NE(bu, nullptr);
+    EXPECT_TRUE(bin_unpack_obj(
-    uint8_t val;
+        [](Bin_Unpack *bu, void *obj) { return bin_unpack_u08(bu, static_cast<uint8_t *>(obj)); },
-    ASSERT_TRUE(bin_unpack_u08(bu.get(), &val));
+        &unpacked, buf.data(), buf.size()));
-    EXPECT_EQ(val, 123);
+
+    EXPECT_EQ(unpacked, 123);
 }
 
 TEST(BinPack, LargeMsgPackedUint32CannotBeUnpackedAsUint8)
 {
+    const uint32_t orig = 1234567;
     std::array<uint8_t, 5> buf;
-    Bin_Pack_Ptr bp(bin_pack_new(buf.data(), buf.size()));
+    EXPECT_TRUE(bin_pack_obj(
-    ASSERT_NE(bp, nullptr);
+        [](Bin_Pack *bp, const Logger *logger, const void *obj) {
-    ASSERT_TRUE(bin_pack_u32(bp.get(), 1234567));
+            return bin_pack_u32(bp, *static_cast<const uint32_t *>(obj));
+        },
+        nullptr, &orig, buf.data(), buf.size()));
 
-    Bin_Unpack_Ptr bu(bin_unpack_new(buf.data(), buf.size()));
+    uint8_t unpacked;
-    ASSERT_NE(bu, nullptr);
+    EXPECT_FALSE(bin_unpack_obj(
-    uint8_t val;
+        [](Bin_Unpack *bu, void *obj) { return bin_unpack_u08(bu, static_cast<uint8_t *>(obj)); },
-    EXPECT_FALSE(bin_unpack_u08(bu.get(), &val));
+        &unpacked, buf.data(), buf.size()));
 }
 
 TEST(BinPack, BinCanHoldPackedInts)
 {
-    std::array<uint8_t, 12> buf;
+    struct Stuff {
-    Bin_Pack_Ptr bp(bin_pack_new(buf.data(), buf.size()));
+        uint64_t u64;
-    ASSERT_NE(bp, nullptr);
+        uint16_t u16;
-    ASSERT_TRUE(bin_pack_bin_marker(bp.get(), 8));
+    };
-    ASSERT_TRUE(bin_pack_u64_b(bp.get(), 1234567812345678LL));
+    const Stuff orig = {1234567812345678LL, 54321};
-    ASSERT_TRUE(bin_pack_u16_b(bp.get(), 54321));
+    static const uint32_t packed_size = sizeof(uint64_t) + sizeof(uint16_t);
 
-    Bin_Unpack_Ptr bu(bin_unpack_new(buf.data(), buf.size()));
+    std::array<uint8_t, 12> buf;
-    ASSERT_NE(bu, nullptr);
+    EXPECT_TRUE(bin_pack_obj(
-    uint32_t size;
+        [](Bin_Pack *bp, const Logger *logger, const void *obj) {
-    EXPECT_TRUE(bin_unpack_bin_size(bu.get(), &size));
+            const Stuff *self = static_cast<const Stuff *>(obj);
-    EXPECT_EQ(size, 8);
+            return bin_pack_bin_marker(bp, packed_size)  //
-    uint64_t val1;
+                && bin_pack_u64_b(bp, self->u64)  //
-    EXPECT_TRUE(bin_unpack_u64_b(bu.get(), &val1));
+                && bin_pack_u16_b(bp, self->u16);
-    EXPECT_EQ(val1, 1234567812345678LL);
+        },
-    uint16_t val2;
+        nullptr, &orig, buf.data(), buf.size()));
-    EXPECT_TRUE(bin_unpack_u16_b(bu.get(), &val2));
+
-    EXPECT_EQ(val2, 54321);
+    Stuff unpacked;
+    EXPECT_TRUE(bin_unpack_obj(
+        [](Bin_Unpack *bu, void *obj) {
+            Stuff *stuff = static_cast<Stuff *>(obj);
+            uint32_t size;
+            return bin_unpack_bin_size(bu, &size)  //
+                && size == 10  //
+                && bin_unpack_u64_b(bu, &stuff->u64)  //
+                && bin_unpack_u16_b(bu, &stuff->u16);
+        },
+        &unpacked, buf.data(), buf.size()));
+    EXPECT_EQ(unpacked.u64, 1234567812345678LL);
+    EXPECT_EQ(unpacked.u16, 54321);
 }
 
 TEST(BinPack, BinCanHoldArbitraryData)
 {
     std::array<uint8_t, 7> buf;
-    Bin_Pack_Ptr bp(bin_pack_new(buf.data(), buf.size()));
+    EXPECT_TRUE(bin_pack_obj(
-    ASSERT_NE(bp, nullptr);
+        [](Bin_Pack *bp, const Logger *logger, const void *obj) {
-    ASSERT_TRUE(bin_pack_bin_marker(bp.get(), 5));
+            return bin_pack_bin_marker(bp, 5)  //
-    ASSERT_TRUE(bin_pack_bin_b(bp.get(), reinterpret_cast<const uint8_t *>("hello"), 5));
+                && bin_pack_bin_b(bp, reinterpret_cast<const uint8_t *>("hello"), 5);
+        },
+        nullptr, nullptr, buf.data(), buf.size()));
 
-    Bin_Unpack_Ptr bu(bin_unpack_new(buf.data(), buf.size()));
-    ASSERT_NE(bu, nullptr);
     std::array<uint8_t, 5> str;
-    EXPECT_TRUE(bin_unpack_bin_fixed(bu.get(), str.data(), str.size()));
+    EXPECT_TRUE(bin_unpack_obj(
+        [](Bin_Unpack *bu, void *obj) {
+            uint8_t *data = static_cast<uint8_t *>(obj);
+            return bin_unpack_bin_fixed(bu, data, 5);
+        },
+        str.data(), buf.data(), buf.size()));
     EXPECT_EQ(str, (std::array<uint8_t, 5>{'h', 'e', 'l', 'l', 'o'}));
 }
 
@@ -126,9 +149,13 @@ TEST(BinPack, OversizedArrayFailsUnpack)
 {
     std::array<uint8_t, 1> buf = {0x91};
 
-    Bin_Unpack_Ptr bu(bin_unpack_new(buf.data(), buf.size()));
     uint32_t size;
-    EXPECT_FALSE(bin_unpack_array(bu.get(), &size));
+    EXPECT_FALSE(bin_unpack_obj(
+        [](Bin_Unpack *bu, void *obj) {
+            uint32_t *size_ptr = static_cast<uint32_t *>(obj);
+            return bin_unpack_array(bu, size_ptr);
+        },
+        &size, buf.data(), buf.size()));
 }
 
 }  // namespace

toxcore/bin_unpack.c

@@ -51,21 +51,19 @@ static size_t null_writer(cmp_ctx_t *ctx, const void *data, size_t count)
     return 0;
 }
 
-Bin_Unpack *bin_unpack_new(const uint8_t *buf, uint32_t buf_size)
+non_null()
+static void bin_unpack_init(Bin_Unpack *bu, const uint8_t *buf, uint32_t buf_size)
 {
-    Bin_Unpack *bu = (Bin_Unpack *)calloc(1, sizeof(Bin_Unpack));
-    if (bu == nullptr) {
-        return nullptr;
-    }
     bu->bytes = buf;
     bu->bytes_size = buf_size;
     cmp_init(&bu->ctx, bu, buf_reader, buf_skipper, null_writer);
-    return bu;
 }
 
-void bin_unpack_free(Bin_Unpack *bu)
+bool bin_unpack_obj(bin_unpack_cb *callback, void *obj, const uint8_t *buf, uint32_t buf_size)
 {
-    free(bu);
+    Bin_Unpack bu;
+    bin_unpack_init(&bu, buf, buf_size);
+    return callback(&bu, obj);
 }
 
 bool bin_unpack_array(Bin_Unpack *bu, uint32_t *size)

toxcore/bin_unpack.h

@@ -16,25 +16,34 @@ extern "C" {
 
 /**
  * @brief Binary deserialisation object.
+ *
+ * User code never creates this object. It is created and destroyed within the below functions,
+ * and passed to the callback. This enforces an alloc/dealloc bracket, so user code can never
+ * forget to clean up an unpacker.
  */
 typedef struct Bin_Unpack Bin_Unpack;
 
-/** @brief Allocate a new unpacker object.
+/** @brief Function used to unpack an object.
  *
- * @param buf The byte array to unpack values from.
+ * This function would typically cast the `void *` to the actual object pointer type and then call
+ * more appropriately typed unpacking functions.
+ */
+typedef bool bin_unpack_cb(Bin_Unpack *bu, void *obj);
+
+/** @brief Unpack an object from a buffer of a given size.
+ *
+ * This function creates and initialises a `Bin_Unpack` object, calls the callback with the
+ * unpacker object and the to-be-unpacked object, and then cleans up the unpacker object.
+ *
+ * @param callback The function called on the created unpacker and unpacked object.
+ * @param obj The object to be packed, passed as `obj` to the callback.
+ * @param buf A byte array containing the serialised representation of `obj`.
  * @param buf_size The size of the byte array.
  *
- * @retval nullptr on allocation failure.
+ * @retval false if an error occurred (e.g. buffer overrun).
  */
 non_null()
-Bin_Unpack *bin_unpack_new(const uint8_t *buf, uint32_t buf_size);
+bool bin_unpack_obj(bin_unpack_cb *callback, void *obj, const uint8_t *buf, uint32_t buf_size);
-
-/** @brief Deallocates an unpacker object.
- *
- * Does not deallocate the buffer inside.
- */
-nullable(1)
-void bin_unpack_free(Bin_Unpack *bu);
 
 /** @brief Start unpacking a MessagePack array.
  *

toxcore/events/events_alloc.h

@@ -69,12 +69,6 @@ tox_group_self_join_cb tox_events_handle_group_self_join;
 tox_group_join_fail_cb tox_events_handle_group_join_fail;
 tox_group_moderation_cb tox_events_handle_group_moderation;
 
-non_null(2) nullable(1)
-bool tox_events_pack(const Tox_Events *events, Bin_Pack *bp);
-
-non_null()
-bool tox_events_unpack(Tox_Events *events, Bin_Unpack *bu, const Memory *mem);
-
 non_null()
 Tox_Events_State *tox_events_alloc(void *user_data);
 

toxcore/tox_events.c

@@ -1,5 +1,5 @@
 /* SPDX-License-Identifier: GPL-3.0-or-later
- * Copyright © 2022 The TokTok team.
+ * Copyright © 2022-2024 The TokTok team.
  */
 
 #include "tox_events.h"
@@ -102,14 +102,20 @@ Tox_Events *tox_events_iterate(Tox *tox, bool fail_hard, Tox_Err_Events_Iterate
     return state.events;
 }
 
-bool tox_events_pack(const Tox_Events *events, Bin_Pack *bp)
+non_null(1) nullable(2, 3)
+static bool tox_events_pack(Bin_Pack *bp, const Logger *logger, const void *obj)
 {
-    const uint32_t size = tox_events_get_size(events);
+    const Tox_Events *events = (const Tox_Events *)obj;
-    if (!bin_pack_array(bp, size)) {
+
+    if (events == nullptr) {
+        return bin_pack_array(bp, 0);
+    }
+
+    if (!bin_pack_array(bp, events->events_size)) {
         return false;
     }
 
-    for (uint32_t i = 0; i < size; ++i) {
+    for (uint32_t i = 0; i < events->events_size; ++i) {
         if (!tox_event_pack(&events->events[i], bp)) {
             return false;
         }
@@ -118,8 +124,11 @@ bool tox_events_pack(const Tox_Events *events, Bin_Pack *bp)
     return true;
 }
 
-bool tox_events_unpack(Tox_Events *events, Bin_Unpack *bu, const Memory *mem)
+non_null()
+static bool tox_events_unpack(Bin_Unpack *bu, void *obj)
 {
+    Tox_Events *events = (Tox_Events *)obj;
+
     uint32_t size;
     if (!bin_unpack_array(bu, &size)) {
         return false;
@@ -127,13 +136,13 @@ bool tox_events_unpack(Tox_Events *events, Bin_Unpack *bu, const Memory *mem)
 
     for (uint32_t i = 0; i < size; ++i) {
         Tox_Event event = {TOX_EVENT_INVALID};
-        if (!tox_event_unpack_into(&event, bu, mem)) {
+        if (!tox_event_unpack_into(&event, bu, events->mem)) {
-            tox_event_destruct(&event, mem);
+            tox_event_destruct(&event, events->mem);
             return false;
         }
 
         if (!tox_events_add(events, &event)) {
-            tox_event_destruct(&event, mem);
+            tox_event_destruct(&event, events->mem);
             return false;
         }
     }
@@ -143,35 +152,21 @@ bool tox_events_unpack(Tox_Events *events, Bin_Unpack *bu, const Memory *mem)
     return true;
 }
 
-non_null(1) nullable(2, 3)
-static bool tox_events_bin_pack_handler(Bin_Pack *bp, const Logger *logger, const void *obj)
-{
-    const Tox_Events *events = (const Tox_Events *)obj;
-    return tox_events_pack(events, bp);
-}
-
 uint32_t tox_events_bytes_size(const Tox_Events *events)
 {
-    return bin_pack_obj_size(tox_events_bin_pack_handler, nullptr, events);
+    return bin_pack_obj_size(tox_events_pack, nullptr, events);
 }
 
 bool tox_events_get_bytes(const Tox_Events *events, uint8_t *bytes)
 {
-    return bin_pack_obj(tox_events_bin_pack_handler, nullptr, events, bytes, UINT32_MAX);
+    return bin_pack_obj(tox_events_pack, nullptr, events, bytes, UINT32_MAX);
 }
 
 Tox_Events *tox_events_load(const Tox_System *sys, const uint8_t *bytes, uint32_t bytes_size)
 {
-    Bin_Unpack *bu = bin_unpack_new(bytes, bytes_size);
-
-    if (bu == nullptr) {
-        return nullptr;
-    }
-
     Tox_Events *events = (Tox_Events *)mem_alloc(sys->mem, sizeof(Tox_Events));
 
     if (events == nullptr) {
-        bin_unpack_free(bu);
         return nullptr;
     }
 
@@ -180,13 +175,11 @@ Tox_Events *tox_events_load(const Tox_System *sys, const uint8_t *bytes, uint32_
     };
     events->mem = sys->mem;
 
-    if (!tox_events_unpack(events, bu, sys->mem)) {
+    if (!bin_unpack_obj(tox_events_unpack, events, bytes, bytes_size)) {
         tox_events_free(events);
-        bin_unpack_free(bu);
         return nullptr;
     }
 
-    bin_unpack_free(bu);
     return events;
 }
 

toxcore/tox_events.h

@@ -1,5 +1,5 @@
 /* SPDX-License-Identifier: GPL-3.0-or-later
- * Copyright © 2022 The TokTok team.
+ * Copyright © 2022-2024 The TokTok team.
  */
 
 #ifndef C_TOXCORE_TOXCORE_TOX_EVENTS_H