From c650d9d34564108f53983f66500a09075989a40a Mon Sep 17 00:00:00 2001 From: iphydf Date: Tue, 16 Jan 2024 19:26:29 +0000 Subject: [PATCH] refactor: Pass `this` pointer as first param to s11n callbacks. --- .../docker/tox-bootstrapd.sha256 | 2 +- toxcore/DHT.c | 12 +++--- toxcore/Messenger.c | 10 ++--- toxcore/bin_pack.c | 23 ++++++---- toxcore/bin_pack.h | 43 +++++++++++++------ toxcore/bin_pack_test.cc | 40 ++++++++--------- toxcore/bin_unpack.c | 2 +- toxcore/bin_unpack.h | 5 ++- toxcore/tox_events.c | 10 ++--- 9 files changed, 87 insertions(+), 60 deletions(-) diff --git a/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 b/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 index 7dc854aa..f17ebbb5 100644 --- a/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 +++ b/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 @@ -1 +1 @@ -4384074ef96cf8f1ed3c420a58b7f67a49a4e22ad5b8fe1d66a4bddf61235fce /usr/local/bin/tox-bootstrapd +c31bc1cd7249c62b4f1d2c9ea26c9e8528d9b897dc29afbdbe95894d30c92c8d /usr/local/bin/tox-bootstrapd diff --git a/toxcore/DHT.c b/toxcore/DHT.c index e95684a9..e4bc1f1c 100644 --- a/toxcore/DHT.c +++ b/toxcore/DHT.c @@ -420,7 +420,7 @@ 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); @@ -428,13 +428,13 @@ static bool bin_pack_ip_port_handler(Bin_Pack *bp, const Logger *logger, const v 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; } @@ -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) @@ -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; diff --git a/toxcore/Messenger.c b/toxcore/Messenger.c index 920b8793..3793adf9 100644 --- a/toxcore/Messenger.c +++ b/toxcore/Messenger.c @@ -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); @@ -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() @@ -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; } @@ -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; diff --git a/toxcore/bin_pack.c b/toxcore/bin_pack.c index 85d09736..76d47820 100644 --- a/toxcore/bin_pack.c +++ b/toxcore/bin_pack.c @@ -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); + } 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); + } for (uint32_t i = 0; i < arr_size; ++i) { - if (!callback(&bp, logger, arr, i)) { + if (!callback(arr, i, logger, &bp)) { return false; } } @@ -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; } - diff --git a/toxcore/bin_pack.h b/toxcore/bin_pack.h index 544bb7f4..0de0b116 100644 --- a/toxcore/bin_pack.h +++ b/toxcore/bin_pack.h @@ -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; @@ -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. * @@ -34,19 +49,19 @@ 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. * * @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. * * @return The packed size of the passed object according to the callback. * @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. * @@ -56,16 +71,18 @@ 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. * @@ -73,15 +90,15 @@ bool bin_pack_obj(bin_pack_cb *callback, const Logger *logger, const void *obj, * * @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. * @@ -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. * diff --git a/toxcore/bin_pack_test.cc b/toxcore/bin_pack_test.cc index 92196d1d..43959424 100644 --- a/toxcore/bin_pack_test.cc +++ b/toxcore/bin_pack_test.cc @@ -15,10 +15,10 @@ TEST(BinPack, TooSmallBufferIsNotExceeded) const uint64_t orig = 1234567812345678LL; std::array 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(obj)); }, - nullptr, &orig, buf.data(), buf.size())); + &orig, nullptr, buf.data(), buf.size())); } TEST(BinPack, PackedUint64CanBeUnpacked) @@ -26,14 +26,14 @@ TEST(BinPack, PackedUint64CanBeUnpacked) const uint64_t orig = 1234567812345678LL; std::array 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(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(obj)); }, &unpacked, buf.data(), buf.size())); @@ -45,14 +45,14 @@ TEST(BinPack, MsgPackedUint8CanBeUnpackedAsUint32) const uint8_t orig = 123; std::array 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(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(obj)); }, + [](void *obj, Bin_Unpack *bu) { return bin_unpack_u32(bu, static_cast(obj)); }, &unpacked, buf.data(), buf.size())); EXPECT_EQ(unpacked, 123); } @@ -62,14 +62,14 @@ TEST(BinPack, MsgPackedUint32CanBeUnpackedAsUint8IfSmallEnough) const uint32_t orig = 123; std::array 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(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(obj)); }, + [](void *obj, Bin_Unpack *bu) { return bin_unpack_u08(bu, static_cast(obj)); }, &unpacked, buf.data(), buf.size())); EXPECT_EQ(unpacked, 123); @@ -80,14 +80,14 @@ TEST(BinPack, LargeMsgPackedUint32CannotBeUnpackedAsUint8) const uint32_t orig = 1234567; std::array 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(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(obj)); }, + [](void *obj, Bin_Unpack *bu) { return bin_unpack_u08(bu, static_cast(obj)); }, &unpacked, buf.data(), buf.size())); } @@ -102,17 +102,17 @@ TEST(BinPack, BinCanHoldPackedInts) std::array 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(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(obj); uint32_t size; return bin_unpack_bin_size(bu, &size) // @@ -129,7 +129,7 @@ TEST(BinPack, BinCanHoldArbitraryData) { std::array 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("hello"), 5); }, @@ -137,7 +137,7 @@ TEST(BinPack, BinCanHoldArbitraryData) std::array str; EXPECT_TRUE(bin_unpack_obj( - [](Bin_Unpack *bu, void *obj) { + [](void *obj, Bin_Unpack *bu) { uint8_t *data = static_cast(obj); return bin_unpack_bin_fixed(bu, data, 5); }, @@ -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(obj); return bin_unpack_array(bu, size_ptr); }, diff --git a/toxcore/bin_unpack.c b/toxcore/bin_unpack.c index 3fb4ac74..860ff3e5 100644 --- a/toxcore/bin_unpack.c +++ b/toxcore/bin_unpack.c @@ -63,7 +63,7 @@ bool bin_unpack_obj(bin_unpack_cb *callback, void *obj, const uint8_t *buf, uint { Bin_Unpack bu; bin_unpack_init(&bu, buf, buf_size); - return callback(&bu, obj); + return callback(obj, &bu); } bool bin_unpack_array(Bin_Unpack *bu, uint32_t *size) diff --git a/toxcore/bin_unpack.h b/toxcore/bin_unpack.h index e3cec3de..b5ae4ece 100644 --- a/toxcore/bin_unpack.h +++ b/toxcore/bin_unpack.h @@ -28,13 +28,16 @@ typedef struct Bin_Unpack Bin_Unpack; * 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); +typedef bool bin_unpack_cb(void *obj, Bin_Unpack *bu); /** @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. * + * Unlike `bin_pack_obj`, this function does not support NULL anywhere. The input array + * must be non-null, even if it is zero-length. + * * @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`. diff --git a/toxcore/tox_events.c b/toxcore/tox_events.c index 62d7d3e1..27efeaa8 100644 --- a/toxcore/tox_events.c +++ b/toxcore/tox_events.c @@ -102,8 +102,8 @@ Tox_Events *tox_events_iterate(Tox *tox, bool fail_hard, Tox_Err_Events_Iterate return state.events; } -non_null(1) nullable(2, 3) -static bool tox_events_pack(Bin_Pack *bp, const Logger *logger, const void *obj) +non_null(3) nullable(1, 2) +static bool tox_events_pack(const void *obj, const Logger *logger, Bin_Pack *bp) { const Tox_Events *events = (const Tox_Events *)obj; @@ -125,7 +125,7 @@ static bool tox_events_pack(Bin_Pack *bp, const Logger *logger, const void *obj) } non_null() -static bool tox_events_unpack(Bin_Unpack *bu, void *obj) +static bool tox_events_unpack(void *obj, Bin_Unpack *bu) { Tox_Events *events = (Tox_Events *)obj; @@ -154,12 +154,12 @@ static bool tox_events_unpack(Bin_Unpack *bu, void *obj) uint32_t tox_events_bytes_size(const Tox_Events *events) { - return bin_pack_obj_size(tox_events_pack, nullptr, events); + return bin_pack_obj_size(tox_events_pack, events, nullptr); } bool tox_events_get_bytes(const Tox_Events *events, uint8_t *bytes) { - return bin_pack_obj(tox_events_pack, nullptr, events, bytes, UINT32_MAX); + return bin_pack_obj(tox_events_pack, events, nullptr, bytes, UINT32_MAX); } Tox_Events *tox_events_load(const Tox_System *sys, const uint8_t *bytes, uint32_t bytes_size)