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.

Author: iphydf

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);
-    if (!bin_pack_obj_array(bin_pack_node_handler, logger, nodes, number, data, length)) {
+    const uint32_t size = bin_pack_obj_array_b_size(bin_pack_node_handler, logger, nodes, number);
+    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);
-    if (bu == nullptr) {
-        LOGGER_ERROR(m->log, "failed to allocate binary unpacker");
-        return STATE_LOAD_STATUS_ERROR;
-    }
+    Messenger *m = (Messenger *)obj;
 
     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 STATE_LOAD_STATUS_ERROR;
+        return false;
     }
 
-    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;
-    return count;
+    bp->bytes_pos += data_size;
+    return data_size;
 }
 
 non_null(1) nullable(2)
@@ -80,45 +79,30 @@ 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;
         }
     }
     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;
         }
     }
     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
- * `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`.
+ * Behaves exactly like `bin_pack_obj_b_array` but doesn't write.
  *
  * @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
- * `count`. This function is here just so we don't need to write the same
+ * Calls the callback `arr_size` times with increasing `index` argument from 0 to
+ * `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);
-
-/** @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);
+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 Start packing a MessagePack array.
  *