Skip to content

Commit

Permalink
cleanup: Remove bin_pack_{new,free}.
Browse files Browse the repository at this point in the history
We should only ever use `bin_pack_obj` and friends, which stack-allocate
the packer and pass it to callbacks.
  • Loading branch information
iphydf committed Jan 16, 2024
1 parent 21a8ff5 commit de4cbb6
Show file tree
Hide file tree
Showing 10 changed files with 177 additions and 190 deletions.
4 changes: 2 additions & 2 deletions toxcore/DHT.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
24 changes: 14 additions & 10 deletions toxcore/Messenger.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
}
Expand Down
20 changes: 2 additions & 18 deletions toxcore/bin_pack.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#include "bin_pack.h"

#include <assert.h>
#include <stdlib.h>
#include <string.h>

#include "../third_party/cmp/cmp.h"
Expand Down Expand Up @@ -80,7 +79,7 @@ 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 count)
{
Bin_Pack bp;
bin_pack_init(&bp, nullptr, 0);
Expand All @@ -92,7 +91,7 @@ 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 count, uint8_t *buf, uint32_t buf_size)
{
Bin_Pack bp;
bin_pack_init(&bp, buf, buf_size);
Expand All @@ -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);
Expand Down
36 changes: 7 additions & 29 deletions toxcore/bin_pack.h
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
*
Expand All @@ -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.
*
Expand Down
Loading

0 comments on commit de4cbb6

Please sign in to comment.