Skip to content

Commit

Permalink
refactor: Pass this pointer as first param to s11n callbacks.
Browse files Browse the repository at this point in the history
  • Loading branch information
iphydf committed Jan 16, 2024
1 parent 259de48 commit 663dbbb
Show file tree
Hide file tree
Showing 9 changed files with 86 additions and 59 deletions.
2 changes: 1 addition & 1 deletion other/bootstrap_daemon/docker/tox-bootstrapd.sha256
Original file line number Diff line number Diff line change
@@ -1 +1 @@
189633f3accb67886c402bf242616d9b3e8258f8050dbd00a10b7c6147ed28aa /usr/local/bin/tox-bootstrapd
059e11465902804feafed5d97fd05a0e46e1c5b76493336caa4ad0f4175f8c7d /usr/local/bin/tox-bootstrapd
12 changes: 6 additions & 6 deletions toxcore/DHT.c
Original file line number Diff line number Diff line change
Expand Up @@ -420,21 +420,21 @@ static bool bin_pack_ip_port(Bin_Pack *bp, const Logger *logger, const IP_Port *
}

non_null()
static bool bin_pack_ip_port_handler(Bin_Pack *bp, const Logger *logger, const void *obj)
static bool bin_pack_ip_port_handler(const void *obj, const Logger *logger, Bin_Pack *bp)
{
const IP_Port *ip_port = (const IP_Port *)obj;
return bin_pack_ip_port(bp, logger, ip_port);
}

int pack_ip_port(const Logger *logger, uint8_t *data, uint16_t length, const IP_Port *ip_port)
{
const uint32_t size = bin_pack_obj_size(bin_pack_ip_port_handler, logger, ip_port);
const uint32_t size = bin_pack_obj_size(bin_pack_ip_port_handler, ip_port, logger);

if (size > length) {
return -1;
}

if (!bin_pack_obj(bin_pack_ip_port_handler, logger, ip_port, data, length)) {
if (!bin_pack_obj(bin_pack_ip_port_handler, ip_port, logger, data, length)) {
return -1;
}

Expand Down Expand Up @@ -543,7 +543,7 @@ int unpack_ip_port(IP_Port *ip_port, const uint8_t *data, uint16_t length, bool
* @retval true on success.
*/
non_null()
static bool bin_pack_node_handler(Bin_Pack *bp, const Logger *logger, const void *arr, uint32_t index)
static bool bin_pack_node_handler(const void *arr, uint32_t index, const Logger *logger, Bin_Pack *bp)
{
const Node_format *nodes = (const Node_format *)arr;
return bin_pack_ip_port(bp, logger, &nodes[index].ip_port)
Expand All @@ -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_b_size(bin_pack_node_handler, logger, nodes, number);
if (!bin_pack_obj_array_b(bin_pack_node_handler, logger, nodes, number, data, length)) {
const uint32_t size = bin_pack_obj_array_b_size(bin_pack_node_handler, nodes, number, logger);
if (!bin_pack_obj_array_b(bin_pack_node_handler, nodes, number, logger, data, length)) {
return -1;
}
return size;
Expand Down
10 changes: 5 additions & 5 deletions toxcore/Messenger.c
Original file line number Diff line number Diff line change
Expand Up @@ -3191,7 +3191,7 @@ static void pack_groupchats(const GC_Session *c, Bin_Pack *bp)
}

non_null()
static bool pack_groupchats_handler(Bin_Pack *bp, const Logger *log, const void *obj)
static bool pack_groupchats_handler(const void *obj, const Logger *log, Bin_Pack *bp)
{
const GC_Session *session = (const GC_Session *)obj;
pack_groupchats(session, bp);
Expand All @@ -3201,8 +3201,8 @@ static bool pack_groupchats_handler(Bin_Pack *bp, const Logger *log, const void
non_null()
static uint32_t saved_groups_size(const Messenger *m)
{
const GC_Session *c = m->group_handler;
return bin_pack_obj_size(pack_groupchats_handler, m->log, c);
const GC_Session *session = m->group_handler;
return bin_pack_obj_size(pack_groupchats_handler, session, m->log);
}

non_null()
Expand All @@ -3224,7 +3224,7 @@ static uint8_t *groups_save(const Messenger *m, uint8_t *data)

data = state_write_section_header(data, STATE_COOKIE_TYPE, len, STATE_TYPE_GROUPS);

if (!bin_pack_obj(pack_groupchats_handler, m->log, c, data, len)) {
if (!bin_pack_obj(pack_groupchats_handler, c, m->log, data, len)) {
LOGGER_FATAL(m->log, "failed to pack group chats into buffer of length %u", len);
return data;
}
Expand All @@ -3237,7 +3237,7 @@ static uint8_t *groups_save(const Messenger *m, uint8_t *data)
}

non_null()
static bool handle_groups_load(Bin_Unpack *bu, void *obj)
static bool handle_groups_load(void *obj, Bin_Unpack *bu)
{
Messenger *m = (Messenger *)obj;

Expand Down
23 changes: 14 additions & 9 deletions toxcore/bin_pack.c
Original file line number Diff line number Diff line change
Expand Up @@ -62,41 +62,47 @@ static void bin_pack_init(Bin_Pack *bp, uint8_t *buf, uint32_t buf_size)
cmp_init(&bp->ctx, bp, null_reader, null_skipper, buf_writer);
}

uint32_t bin_pack_obj_size(bin_pack_cb *callback, const Logger *logger, const void *obj)
uint32_t bin_pack_obj_size(bin_pack_cb *callback, const void *obj, const Logger *logger)
{
Bin_Pack bp;
bin_pack_init(&bp, nullptr, 0);
if (!callback(&bp, logger, obj)) {
if (!callback(obj, logger, &bp)) {
return UINT32_MAX;
}
return bp.bytes_pos;
}

bool bin_pack_obj(bin_pack_cb *callback, const Logger *logger, const void *obj, uint8_t *buf, uint32_t buf_size)
bool bin_pack_obj(bin_pack_cb *callback, const void *obj, const Logger *logger, uint8_t *buf, uint32_t buf_size)
{
Bin_Pack bp;
bin_pack_init(&bp, buf, buf_size);
return callback(&bp, logger, obj);
return callback(obj, logger, &bp);
}

uint32_t bin_pack_obj_array_b_size(bin_pack_array_cb *callback, const Logger *logger, const void *arr, uint32_t arr_size)
uint32_t bin_pack_obj_array_b_size(bin_pack_array_cb *callback, const void *arr, uint32_t arr_size, const Logger *logger)
{
Bin_Pack bp;
bin_pack_init(&bp, nullptr, 0);
if (arr == nullptr) {
assert(arr_size == 0);

Check warning on line 87 in toxcore/bin_pack.c

View check run for this annotation

Codecov / codecov/patch

toxcore/bin_pack.c#L87

Added line #L87 was not covered by tests
}
for (uint32_t i = 0; i < arr_size; ++i) {
if (!callback(&bp, logger, arr, i)) {
if (!callback(arr, i, logger, &bp)) {
return UINT32_MAX;
}
}
return bp.bytes_pos;
}

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)
bool bin_pack_obj_array_b(bin_pack_array_cb *callback, const void *arr, uint32_t arr_size, const Logger *logger, uint8_t *buf, uint32_t buf_size)
{
Bin_Pack bp;
bin_pack_init(&bp, buf, buf_size);
if (arr == nullptr) {
assert(arr_size == 0);

Check warning on line 102 in toxcore/bin_pack.c

View check run for this annotation

Codecov / codecov/patch

toxcore/bin_pack.c#L102

Added line #L102 was not covered by tests
}
for (uint32_t i = 0; i < arr_size; ++i) {
if (!callback(&bp, logger, arr, i)) {
if (!callback(arr, i, logger, &bp)) {
return false;
}
}
Expand Down Expand Up @@ -175,4 +181,3 @@ bool bin_pack_bin_b(Bin_Pack *bp, const uint8_t *data, uint32_t length)
{
return bp->ctx.write(&bp->ctx, data, length) == length;
}

41 changes: 30 additions & 11 deletions toxcore/bin_pack.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,21 @@ extern "C" {

/**
* @brief Binary serialisation object.
*
* Some notes on parameter order:
*
* - We pass the `obj` pointer as `this`-like pointer first to the callbacks.
* - Any extra arguments passed to the callback follow the `obj` (and in case of
* array packing, the `arr` and `arr_size` parameters).
* - The packer is passed last.
*
* This roughly matches a curried lambda function:
*
* @code
* bin_pack_obj([](const void *obj, const Logger *logger, Bin_Pack *bp) { ... }, obj, logger, buf, buf_size);
* // Translates roughly to:
* bin_pack_obj([obj, logger](Bin_Pack *bp) { ... }, buf, buf_size);
* @endcode
*/
typedef struct Bin_Pack Bin_Pack;

Expand All @@ -24,7 +39,7 @@ typedef struct Bin_Pack Bin_Pack;
* This function would typically cast the `void *` to the actual object pointer type and then call
* more appropriately typed packing functions.
*/
typedef bool bin_pack_cb(Bin_Pack *bp, const Logger *logger, const void *obj);
typedef bool bin_pack_cb(const void *obj, const Logger *logger, Bin_Pack *bp);

/** @brief Function used to pack an array of objects.
*
Expand All @@ -34,7 +49,7 @@ typedef bool bin_pack_cb(Bin_Pack *bp, const Logger *logger, const void *obj);
* @param arr is the object array as void pointer.
* @param index is the index in the object array that is currently being packed.
*/
typedef bool bin_pack_array_cb(Bin_Pack *bp, const Logger *logger, const void *arr, uint32_t index);
typedef bool bin_pack_array_cb(const void *arr, uint32_t index, const Logger *logger, Bin_Pack *bp);

/** @brief Determine the serialised size of an object.
*
Expand All @@ -46,7 +61,7 @@ typedef bool bin_pack_array_cb(Bin_Pack *bp, const Logger *logger, const void *a
* @retval UINT32_MAX in case of errors such as buffer overflow.
*/
non_null(1) nullable(2, 3)
uint32_t bin_pack_obj_size(bin_pack_cb *callback, const Logger *logger, const void *obj);
uint32_t bin_pack_obj_size(bin_pack_cb *callback, const void *obj, const Logger *logger);

/** @brief Pack an object into a buffer of a given size.
*
Expand All @@ -56,32 +71,34 @@ uint32_t bin_pack_obj_size(bin_pack_cb *callback, const Logger *logger, const vo
* You can use `bin_pack_obj_size` to determine the minimum required size of `buf`. If packing
* overflows `uint32_t`, this function returns `false`.
*
* Passing NULL for `obj` is supported, but requires that the callback supports nullable inputs.
*
* @param callback The function called on the created packer and packed object.
* @param logger Optional logger object to pass to the callback.
* @param obj The object to be packed, passed as `obj` to the callback.
* @param logger Optional logger object to pass to the callback.
* @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 false if an error occurred (e.g. buffer overflow).
*/
non_null(1, 4) nullable(2, 3)
bool bin_pack_obj(bin_pack_cb *callback, const Logger *logger, const void *obj, uint8_t *buf, uint32_t buf_size);
bool bin_pack_obj(bin_pack_cb *callback, const void *obj, const Logger *logger, uint8_t *buf, uint32_t buf_size);

/** @brief Determine the serialised size of an object array.
*
* 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 arr_size The number of elements in the object array.
* @param logger Optional logger object to pass to the callback.
*
* @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_b_size(bin_pack_array_cb *callback, const Logger *logger, const void *arr, uint32_t arr_size);
non_null(1) nullable(2, 4)
uint32_t bin_pack_obj_array_b_size(bin_pack_array_cb *callback, const void *arr, uint32_t arr_size, const Logger *logger);

/** @brief Pack an object array into a buffer of a given size.
*
Expand All @@ -93,18 +110,20 @@ uint32_t bin_pack_obj_array_b_size(bin_pack_array_cb *callback, const Logger *lo
* Similar to `bin_pack_obj` but for arrays. Does not write the array length, so
* if you need that, write it manually using `bin_pack_array`.
*
* Passing NULL for `arr` has no effect, but requires that `arr_size` is 0.
*
* @param callback The function called on the created packer and packed object
* 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 arr_size The number of elements in the object array.
* @param logger Optional logger object to pass to the callback.
* @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_b(bin_pack_array_cb *callback, const Logger *logger, const void *arr, uint32_t arr_size, uint8_t *buf, uint32_t buf_size);
non_null(1, 5) nullable(2, 4)
bool bin_pack_obj_array_b(bin_pack_array_cb *callback, const void *arr, uint32_t arr_size, const Logger *logger, uint8_t *buf, uint32_t buf_size);

/** @brief Start packing a MessagePack array.
*
Expand Down
40 changes: 20 additions & 20 deletions toxcore/bin_pack_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,25 +15,25 @@ TEST(BinPack, TooSmallBufferIsNotExceeded)
const uint64_t orig = 1234567812345678LL;
std::array<uint8_t, sizeof(orig) - 1> buf;
EXPECT_FALSE(bin_pack_obj(
[](Bin_Pack *bp, const Logger *logger, const void *obj) {
[](const void *obj, const Logger *logger, Bin_Pack *bp) {
return bin_pack_u64_b(bp, *static_cast<const uint64_t *>(obj));
},
nullptr, &orig, buf.data(), buf.size()));
&orig, nullptr, buf.data(), buf.size()));
}

TEST(BinPack, PackedUint64CanBeUnpacked)
{
const uint64_t orig = 1234567812345678LL;
std::array<uint8_t, 8> buf;
EXPECT_TRUE(bin_pack_obj(
[](Bin_Pack *bp, const Logger *logger, const void *obj) {
[](const void *obj, const Logger *logger, Bin_Pack *bp) {
return bin_pack_u64_b(bp, *static_cast<const uint64_t *>(obj));
},
nullptr, &orig, buf.data(), buf.size()));
&orig, nullptr, buf.data(), buf.size()));

uint64_t unpacked;
EXPECT_TRUE(bin_unpack_obj(
[](Bin_Unpack *bu, void *obj) {
[](void *obj, Bin_Unpack *bu) {
return bin_unpack_u64_b(bu, static_cast<uint64_t *>(obj));
},
&unpacked, buf.data(), buf.size()));
Expand All @@ -45,14 +45,14 @@ TEST(BinPack, MsgPackedUint8CanBeUnpackedAsUint32)
const uint8_t orig = 123;
std::array<uint8_t, 2> buf;
EXPECT_TRUE(bin_pack_obj(
[](Bin_Pack *bp, const Logger *logger, const void *obj) {
[](const void *obj, const Logger *logger, Bin_Pack *bp) {
return bin_pack_u08(bp, *static_cast<const uint8_t *>(obj));
},
nullptr, &orig, buf.data(), buf.size()));
&orig, nullptr, buf.data(), buf.size()));

uint32_t unpacked;
EXPECT_TRUE(bin_unpack_obj(
[](Bin_Unpack *bu, void *obj) { return bin_unpack_u32(bu, static_cast<uint32_t *>(obj)); },
[](void *obj, Bin_Unpack *bu) { return bin_unpack_u32(bu, static_cast<uint32_t *>(obj)); },
&unpacked, buf.data(), buf.size()));
EXPECT_EQ(unpacked, 123);
}
Expand All @@ -62,14 +62,14 @@ TEST(BinPack, MsgPackedUint32CanBeUnpackedAsUint8IfSmallEnough)
const uint32_t orig = 123;
std::array<uint8_t, 2> buf;
EXPECT_TRUE(bin_pack_obj(
[](Bin_Pack *bp, const Logger *logger, const void *obj) {
[](const void *obj, const Logger *logger, Bin_Pack *bp) {
return bin_pack_u32(bp, *static_cast<const uint32_t *>(obj));
},
nullptr, &orig, buf.data(), buf.size()));
&orig, nullptr, buf.data(), buf.size()));

uint8_t unpacked;
EXPECT_TRUE(bin_unpack_obj(
[](Bin_Unpack *bu, void *obj) { return bin_unpack_u08(bu, static_cast<uint8_t *>(obj)); },
[](void *obj, Bin_Unpack *bu) { return bin_unpack_u08(bu, static_cast<uint8_t *>(obj)); },
&unpacked, buf.data(), buf.size()));

EXPECT_EQ(unpacked, 123);
Expand All @@ -80,14 +80,14 @@ TEST(BinPack, LargeMsgPackedUint32CannotBeUnpackedAsUint8)
const uint32_t orig = 1234567;
std::array<uint8_t, 5> buf;
EXPECT_TRUE(bin_pack_obj(
[](Bin_Pack *bp, const Logger *logger, const void *obj) {
[](const void *obj, const Logger *logger, Bin_Pack *bp) {
return bin_pack_u32(bp, *static_cast<const uint32_t *>(obj));
},
nullptr, &orig, buf.data(), buf.size()));
&orig, nullptr, buf.data(), buf.size()));

uint8_t unpacked;
EXPECT_FALSE(bin_unpack_obj(
[](Bin_Unpack *bu, void *obj) { return bin_unpack_u08(bu, static_cast<uint8_t *>(obj)); },
[](void *obj, Bin_Unpack *bu) { return bin_unpack_u08(bu, static_cast<uint8_t *>(obj)); },
&unpacked, buf.data(), buf.size()));
}

Expand All @@ -102,17 +102,17 @@ TEST(BinPack, BinCanHoldPackedInts)

std::array<uint8_t, 12> buf;
EXPECT_TRUE(bin_pack_obj(
[](Bin_Pack *bp, const Logger *logger, const void *obj) {
[](const void *obj, const Logger *logger, Bin_Pack *bp) {
const Stuff *self = static_cast<const Stuff *>(obj);
return bin_pack_bin_marker(bp, packed_size) //
&& bin_pack_u64_b(bp, self->u64) //
&& bin_pack_u16_b(bp, self->u16);
},
nullptr, &orig, buf.data(), buf.size()));
&orig, nullptr, buf.data(), buf.size()));

Stuff unpacked;
EXPECT_TRUE(bin_unpack_obj(
[](Bin_Unpack *bu, void *obj) {
[](void *obj, Bin_Unpack *bu) {
Stuff *stuff = static_cast<Stuff *>(obj);
uint32_t size;
return bin_unpack_bin_size(bu, &size) //
Expand All @@ -129,15 +129,15 @@ TEST(BinPack, BinCanHoldArbitraryData)
{
std::array<uint8_t, 7> buf;
EXPECT_TRUE(bin_pack_obj(
[](Bin_Pack *bp, const Logger *logger, const void *obj) {
[](const void *obj, const Logger *logger, Bin_Pack *bp) {
return bin_pack_bin_marker(bp, 5) //
&& bin_pack_bin_b(bp, reinterpret_cast<const uint8_t *>("hello"), 5);
},
nullptr, nullptr, buf.data(), buf.size()));

std::array<uint8_t, 5> str;
EXPECT_TRUE(bin_unpack_obj(
[](Bin_Unpack *bu, void *obj) {
[](void *obj, Bin_Unpack *bu) {
uint8_t *data = static_cast<uint8_t *>(obj);
return bin_unpack_bin_fixed(bu, data, 5);
},
Expand All @@ -151,7 +151,7 @@ TEST(BinPack, OversizedArrayFailsUnpack)

uint32_t size;
EXPECT_FALSE(bin_unpack_obj(
[](Bin_Unpack *bu, void *obj) {
[](void *obj, Bin_Unpack *bu) {
uint32_t *size_ptr = static_cast<uint32_t *>(obj);
return bin_unpack_array(bu, size_ptr);
},
Expand Down
Loading

0 comments on commit 663dbbb

Please sign in to comment.