cleanup: Use tox memory for bin_unpack and net_strerror.

Some of the last places where we use malloc.
TokTok · Jan 18, 2025 · 9d8c9a1 · 9d8c9a1
1 parent d9b8fa6
commit 9d8c9a1
Show file tree

Hide file tree

Showing 15 changed files with 139 additions and 96 deletions.
diff --git a/auto_tests/TCP_test.c b/auto_tests/TCP_test.c
@@ -111,13 +111,13 @@ static void test_basic(void)
     free(handshake_plain);
 
     // Sending the handshake
-    ck_assert_msg(net_send(ns, logger, sock, handshake, TCP_CLIENT_HANDSHAKE_SIZE - 1,
+    ck_assert_msg(net_send(ns, mem, logger, sock, handshake, TCP_CLIENT_HANDSHAKE_SIZE - 1,
                            &localhost, nullptr) == TCP_CLIENT_HANDSHAKE_SIZE - 1,
                   "An attempt to send the initial handshake minus last byte failed.");
 
     do_tcp_server_delay(tcp_s, mono_time, 50);
 
-    ck_assert_msg(net_send(ns, logger, sock, handshake + (TCP_CLIENT_HANDSHAKE_SIZE - 1), 1, &localhost, nullptr) == 1,
+    ck_assert_msg(net_send(ns, mem, logger, sock, handshake + (TCP_CLIENT_HANDSHAKE_SIZE - 1), 1, &localhost, nullptr) == 1,
                   "The attempt to send the last byte of handshake failed.");
 
     free(handshake);
@@ -127,7 +127,7 @@ static void test_basic(void)
     // Receiving server response and decrypting it
     uint8_t response[TCP_SERVER_HANDSHAKE_SIZE];
     uint8_t response_plain[TCP_HANDSHAKE_PLAIN_SIZE];
-    ck_assert_msg(net_recv(ns, logger, sock, response, TCP_SERVER_HANDSHAKE_SIZE, &localhost) == TCP_SERVER_HANDSHAKE_SIZE,
+    ck_assert_msg(net_recv(ns, mem, logger, sock, response, TCP_SERVER_HANDSHAKE_SIZE, &localhost) == TCP_SERVER_HANDSHAKE_SIZE,
                   "Could/did not receive a server response to the initial handshake.");
     ret = decrypt_data(mem, self_public_key, f_secret_key, response, response + CRYPTO_NONCE_SIZE,
                        TCP_SERVER_HANDSHAKE_SIZE - CRYPTO_NONCE_SIZE, response_plain);
@@ -156,7 +156,7 @@ static void test_basic(void)
             msg_length = sizeof(r_req) - i;
         }
 
-        ck_assert_msg(net_send(ns, logger, sock, r_req + i, msg_length, &localhost, nullptr) == msg_length,
+        ck_assert_msg(net_send(ns, mem, logger, sock, r_req + i, msg_length, &localhost, nullptr) == msg_length,
                       "Failed to send request after completing the handshake.");
         i += msg_length;
 
@@ -169,7 +169,7 @@ static void test_basic(void)
     const size_t max_packet_size = 4096;
     uint8_t *packet_resp = (uint8_t *)malloc(max_packet_size);
     ck_assert(packet_resp != nullptr);
-    int recv_data_len = net_recv(ns, logger, sock, packet_resp, 2 + 2 + CRYPTO_PUBLIC_KEY_SIZE + CRYPTO_MAC_SIZE, &localhost);
+    int recv_data_len = net_recv(ns, mem, logger, sock, packet_resp, 2 + 2 + CRYPTO_PUBLIC_KEY_SIZE + CRYPTO_MAC_SIZE, &localhost);
     ck_assert_msg(recv_data_len == 2 + 2 + CRYPTO_PUBLIC_KEY_SIZE + CRYPTO_MAC_SIZE,
                   "Failed to receive server response to request. %d", recv_data_len);
     memcpy(&size, packet_resp, 2);
@@ -241,20 +241,20 @@ static struct sec_TCP_con *new_tcp_con(const Logger *logger, const Memory *mem,
     ck_assert_msg(ret == TCP_CLIENT_HANDSHAKE_SIZE - (CRYPTO_PUBLIC_KEY_SIZE + CRYPTO_NONCE_SIZE),
                   "Failed to encrypt the outgoing handshake.");
 
-    ck_assert_msg(net_send(ns, logger, sock, handshake, TCP_CLIENT_HANDSHAKE_SIZE - 1,
+    ck_assert_msg(net_send(ns, mem, logger, sock, handshake, TCP_CLIENT_HANDSHAKE_SIZE - 1,
                            &localhost, nullptr) == TCP_CLIENT_HANDSHAKE_SIZE - 1,
                   "Failed to send the first portion of the handshake to the TCP relay server.");
 
     do_tcp_server_delay(tcp_s, mono_time, 50);
 
-    ck_assert_msg(net_send(ns, logger, sock, handshake + (TCP_CLIENT_HANDSHAKE_SIZE - 1), 1, &localhost, nullptr) == 1,
+    ck_assert_msg(net_send(ns, mem, logger, sock, handshake + (TCP_CLIENT_HANDSHAKE_SIZE - 1), 1, &localhost, nullptr) == 1,
                   "Failed to send last byte of handshake.");
 
     do_tcp_server_delay(tcp_s, mono_time, 50);
 
     uint8_t response[TCP_SERVER_HANDSHAKE_SIZE];
     uint8_t response_plain[TCP_HANDSHAKE_PLAIN_SIZE];
-    ck_assert_msg(net_recv(sec_c->ns, logger, sock, response, TCP_SERVER_HANDSHAKE_SIZE, &localhost) == TCP_SERVER_HANDSHAKE_SIZE,
+    ck_assert_msg(net_recv(sec_c->ns, mem, logger, sock, response, TCP_SERVER_HANDSHAKE_SIZE, &localhost) == TCP_SERVER_HANDSHAKE_SIZE,
                   "Failed to receive server handshake response.");
     ret = decrypt_data(mem, tcp_server_public_key(tcp_s), f_secret_key, response, response + CRYPTO_NONCE_SIZE,
                        TCP_SERVER_HANDSHAKE_SIZE - CRYPTO_NONCE_SIZE, response_plain);
@@ -271,7 +271,7 @@ static void kill_tcp_con(struct sec_TCP_con *con)
     free(con);
 }
 
-static int write_packet_tcp_test_connection(const Logger *logger, struct sec_TCP_con *con, const uint8_t *data,
+static int write_packet_tcp_test_connection(const Logger *logger, const Memory *mem, struct sec_TCP_con *con, const uint8_t *data,
         uint16_t length)
 {
     const uint16_t packet_size = sizeof(uint16_t) + length + CRYPTO_MAC_SIZE;
@@ -291,7 +291,7 @@ static int write_packet_tcp_test_connection(const Logger *logger, struct sec_TCP
     localhost.ip = get_loopback();
     localhost.port = 0;
 
-    ck_assert_msg(net_send(con->ns, logger, con->sock, packet, packet_size, &localhost, nullptr) == packet_size,
+    ck_assert_msg(net_send(con->ns, mem, logger, con->sock, packet, packet_size, &localhost, nullptr) == packet_size,
                   "Failed to send a packet.");
     return 0;
 }
@@ -302,7 +302,7 @@ static int read_packet_sec_tcp(const Logger *logger, struct sec_TCP_con *con, ui
     localhost.ip = get_loopback();
     localhost.port = 0;
 
-    int rlen = net_recv(con->ns, logger, con->sock, data, length, &localhost);
+    int rlen = net_recv(con->ns, con->mem, logger, con->sock, data, length, &localhost);
     ck_assert_msg(rlen == length, "Did not receive packet of correct length. Wanted %i, instead got %i", length, rlen);
     rlen = decrypt_data_symmetric(con->mem, con->shared_key, con->recv_nonce, data + 2, length - 2, data);
     ck_assert_msg(rlen != -1, "Failed to decrypt a received packet from the Relay server.");
@@ -338,9 +338,9 @@ static void test_some(void)
 
     // Sending wrong public keys to test server response.
     memcpy(requ_p + 1, con3->public_key, CRYPTO_PUBLIC_KEY_SIZE);
-    write_packet_tcp_test_connection(logger, con1, requ_p, sizeof(requ_p));
+    write_packet_tcp_test_connection(logger, mem, con1, requ_p, sizeof(requ_p));
     memcpy(requ_p + 1, con1->public_key, CRYPTO_PUBLIC_KEY_SIZE);
-    write_packet_tcp_test_connection(logger, con3, requ_p, sizeof(requ_p));
+    write_packet_tcp_test_connection(logger, mem, con3, requ_p, sizeof(requ_p));
 
     do_tcp_server_delay(tcp_s, mono_time, 50);
 
@@ -362,9 +362,9 @@ static void test_some(void)
 
     const uint8_t test_packet[512] = {16, 17, 16, 86, 99, 127, 255, 189, 78}; // What is this packet????
 
-    write_packet_tcp_test_connection(logger, con3, test_packet, sizeof(test_packet));
-    write_packet_tcp_test_connection(logger, con3, test_packet, sizeof(test_packet));
-    write_packet_tcp_test_connection(logger, con3, test_packet, sizeof(test_packet));
+    write_packet_tcp_test_connection(logger, mem, con3, test_packet, sizeof(test_packet));
+    write_packet_tcp_test_connection(logger, mem, con3, test_packet, sizeof(test_packet));
+    write_packet_tcp_test_connection(logger, mem, con3, test_packet, sizeof(test_packet));
 
     do_tcp_server_delay(tcp_s, mono_time, 50);
 
@@ -388,9 +388,9 @@ static void test_some(void)
     ck_assert_msg(len == sizeof(test_packet), "wrong len %d", len);
     ck_assert_msg(memcmp(data, test_packet, sizeof(test_packet)) == 0, "packet is wrong %u %u %u %u", data[0], data[1],
                   data[sizeof(test_packet) - 2], data[sizeof(test_packet) - 1]);
-    write_packet_tcp_test_connection(logger, con1, test_packet, sizeof(test_packet));
-    write_packet_tcp_test_connection(logger, con1, test_packet, sizeof(test_packet));
-    write_packet_tcp_test_connection(logger, con1, test_packet, sizeof(test_packet));
+    write_packet_tcp_test_connection(logger, mem, con1, test_packet, sizeof(test_packet));
+    write_packet_tcp_test_connection(logger, mem, con1, test_packet, sizeof(test_packet));
+    write_packet_tcp_test_connection(logger, mem, con1, test_packet, sizeof(test_packet));
     do_tcp_server_delay(tcp_s, mono_time, 50);
     len = read_packet_sec_tcp(logger, con3, data, 2 + sizeof(test_packet) + CRYPTO_MAC_SIZE);
     ck_assert_msg(len == sizeof(test_packet), "wrong len %d", len);
@@ -405,8 +405,8 @@ static void test_some(void)
     ck_assert_msg(memcmp(data, test_packet, sizeof(test_packet)) == 0, "packet is wrong %u %u %u %u", data[0], data[1],
                   data[sizeof(test_packet) - 2], data[sizeof(test_packet) - 1]);
 
-    uint8_t ping_packet[1 + sizeof(uint64_t)] = {TCP_PACKET_PING, 8, 6, 9, 67};
-    write_packet_tcp_test_connection(logger, con1, ping_packet, sizeof(ping_packet));
+    const uint8_t ping_packet[1 + sizeof(uint64_t)] = {TCP_PACKET_PING, 8, 6, 9, 67};
+    write_packet_tcp_test_connection(logger, mem, con1, ping_packet, sizeof(ping_packet));
 
     do_tcp_server_delay(tcp_s, mono_time, 50);
 

diff --git a/auto_tests/network_test.c b/auto_tests/network_test.c
@@ -1,7 +1,5 @@
-#include <stdlib.h>
 #include <string.h>
 
-#include "../testing/misc_tools.h"
 #include "../toxcore/network.h"
 #include "check_compat.h"
 
@@ -34,9 +32,10 @@ static void test_addr_resolv_localhost(void)
     bool res = addr_resolve_or_parse_ip(ns, mem, localhost, &ip, nullptr, true);
 
     int error = net_error();
-    char *strerror = net_new_strerror(error);
+    char *strerror = net_new_strerror(mem, error);
+    ck_assert(strerror != nullptr);
     ck_assert_msg(res, "Resolver failed: %d, %s", error, strerror);
-    net_kill_strerror(strerror);
+    net_kill_strerror(mem, strerror);
 
     Ip_Ntoa ip_str;
     ck_assert_msg(net_family_is_ipv4(ip.family), "Expected family TOX_AF_INET, got %u.", ip.family.value);
@@ -57,9 +56,10 @@ static void test_addr_resolv_localhost(void)
     }
 
     error = net_error();
-    strerror = net_new_strerror(error);
+    strerror = net_new_strerror(mem, error);
+    ck_assert(strerror != nullptr);
     ck_assert_msg(res, "Resolver failed: %d, %s", error, strerror);
-    net_kill_strerror(strerror);
+    net_kill_strerror(mem, strerror);
 
     ck_assert_msg(net_family_is_ipv6(ip.family), "Expected family TOX_AF_INET6 (%d), got %u.", TOX_AF_INET6,
                   ip.family.value);
@@ -80,9 +80,10 @@ static void test_addr_resolv_localhost(void)
     ip_reset(&extra);
     res = addr_resolve_or_parse_ip(ns, mem, localhost, &ip, &extra, true);
     error = net_error();
-    strerror = net_new_strerror(error);
+    strerror = net_new_strerror(mem, error);
+    ck_assert(strerror != nullptr);
     ck_assert_msg(res, "Resolver failed: %d, %s", error, strerror);
-    net_kill_strerror(strerror);
+    net_kill_strerror(mem, strerror);
 
 #if USE_IPV6
     ck_assert_msg(net_family_is_ipv6(ip.family), "Expected family TOX_AF_INET6 (%d), got %u.", TOX_AF_INET6,

diff --git a/toxav/rtp.c b/toxav/rtp.c
@@ -678,7 +678,7 @@ static uint32_t rtp_random_u32(void)
     return randombytes_random();
 }
 
-RTPSession *rtp_new(const Logger *log, int payload_type, Tox *tox, ToxAV *toxav, uint32_t friendnumber,
+RTPSession *rtp_new(const Logger *log, const Memory *mem, int payload_type, Tox *tox, ToxAV *toxav, uint32_t friendnumber,
                     BWController *bwc, void *cs, rtp_m_cb *mcb)
 {
     assert(mcb != nullptr);
@@ -704,6 +704,8 @@ RTPSession *rtp_new(const Logger *log, int payload_type, Tox *tox, ToxAV *toxav,
 
     session->ssrc = payload_type == RTP_TYPE_VIDEO ? 0 : rtp_random_u32(); // Zoff: what is this??
     session->payload_type = payload_type;
+    session->log = log;
+    session->mem = mem;
     session->tox = tox;
     session->toxav = toxav;
     session->friend_number = friendnumber;
@@ -773,18 +775,18 @@ void rtp_stop_receiving(Tox *tox)
  * @param error the error from rtp_send_custom_lossy_packet.
  * @param rdata_size The package length to be shown in the log.
  */
-static void rtp_report_error_maybe(const Logger *log, Tox_Err_Friend_Custom_Packet error, uint16_t rdata_size)
+static void rtp_report_error_maybe(const Logger *log, const Memory *mem, Tox_Err_Friend_Custom_Packet error, uint16_t rdata_size)
 {
     if (error != TOX_ERR_FRIEND_CUSTOM_PACKET_OK) {
-        char *netstrerror = net_new_strerror(net_error());
+        char *netstrerror = net_new_strerror(mem, net_error());
         const char *toxerror = tox_err_friend_custom_packet_to_string(error);
         LOGGER_WARNING(log, "RTP send failed (len: %u)! tox error: %s net error: %s",
                        rdata_size, toxerror, netstrerror);
-        net_kill_strerror(netstrerror);
+        net_kill_strerror(mem, netstrerror);
     }
 }
 
-static void rtp_send_piece(const Logger *log, Tox *tox, uint32_t friend_number, const struct RTPHeader *header,
+static void rtp_send_piece(const Logger *log, const Memory *mem, Tox *tox, uint32_t friend_number, const struct RTPHeader *header,
                            const uint8_t *data, uint8_t *rdata, uint16_t length)
 {
     rtp_header_pack(rdata + 1, header);
@@ -795,7 +797,7 @@ static void rtp_send_piece(const Logger *log, Tox *tox, uint32_t friend_number,
     Tox_Err_Friend_Custom_Packet error;
     tox_friend_send_lossy_packet(tox, friend_number, rdata, rdata_size, &error);
 
-    rtp_report_error_maybe(log, error, rdata_size);
+    rtp_report_error_maybe(log, mem, error, rdata_size);
 }
 
 static struct RTPHeader rtp_default_header(const RTPSession *session, uint32_t length, bool is_keyframe)
@@ -868,7 +870,7 @@ int rtp_send_data(const Logger *log, RTPSession *session, const uint8_t *data, u
          * Send the packet in single piece.
          */
         assert(length < UINT16_MAX);
-        rtp_send_piece(log, session->tox, session->friend_number, &header, data, rdata, length);
+        rtp_send_piece(log, session->mem, session->tox, session->friend_number, &header, data, rdata, length);
     } else {
         /*
          * The length is greater than the maximum allowed length (including header)
@@ -878,7 +880,7 @@ int rtp_send_data(const Logger *log, RTPSession *session, const uint8_t *data, u
         uint16_t piece = MAX_CRYPTO_DATA_SIZE - (RTP_HEADER_SIZE + 1);
 
         while ((length - sent) + RTP_HEADER_SIZE + 1 > MAX_CRYPTO_DATA_SIZE) {
-            rtp_send_piece(log, session->tox, session->friend_number, &header, data + sent, rdata, piece);
+            rtp_send_piece(log, session->mem, session->tox, session->friend_number, &header, data + sent, rdata, piece);
 
             sent += piece;
             header.offset_lower = sent;
@@ -889,7 +891,7 @@ int rtp_send_data(const Logger *log, RTPSession *session, const uint8_t *data, u
         piece = length - sent;
 
         if (piece != 0) {
-            rtp_send_piece(log, session->tox, session->friend_number, &header, data + sent, rdata, piece);
+            rtp_send_piece(log, session->mem, session->tox, session->friend_number, &header, data + sent, rdata, piece);
         }
     }
 

diff --git a/toxav/rtp.h b/toxav/rtp.h
@@ -162,6 +162,7 @@ typedef struct RTPSession {
     struct RTPWorkBufferList *work_buffer_list;
     uint8_t  first_packets_counter; /* dismiss first few lost video packets */
     const Logger *log;
+    const Memory *mem;
     Tox *tox;
     ToxAV *toxav;
     uint32_t friend_number;
@@ -192,7 +193,7 @@ size_t rtp_header_pack(uint8_t *rdata, const struct RTPHeader *header);
  */
 size_t rtp_header_unpack(const uint8_t *data, struct RTPHeader *header);
 
-RTPSession *rtp_new(const Logger *log, int payload_type, Tox *tox, ToxAV *toxav, uint32_t friendnumber,
+RTPSession *rtp_new(const Logger *log, const Memory *mem, int payload_type, Tox *tox, ToxAV *toxav, uint32_t friendnumber,
                     BWController *bwc, void *cs, rtp_m_cb *mcb);
 void rtp_kill(const Logger *log, RTPSession *session);
 void rtp_allow_receiving_mark(RTPSession *session);

diff --git a/toxav/toxav.c b/toxav/toxav.c
@@ -87,6 +87,7 @@ typedef struct DecodeTimeStats {
 } DecodeTimeStats;
 
 struct ToxAV {
+    const Memory *mem;
     Logger *log;
     Tox *tox;
     MSISession *msi;
@@ -219,6 +220,7 @@ ToxAV *toxav_new(Tox *tox, Toxav_Err_New *error)
         goto RETURN;
     }
 
+    av->mem = tox->sys.mem;
     av->log = tox->m->log;
     av->tox = tox;
     av->msi = msi_new(av->log, av->tox);
@@ -994,9 +996,9 @@ static Toxav_Err_Send_Frame send_frames(const ToxAV *av, ToxAVCall *call)
                             is_keyframe);
 
         if (res < 0) {
-            char *netstrerror = net_new_strerror(net_error());
+            char *netstrerror = net_new_strerror(av->mem, net_error());
             LOGGER_WARNING(av->log, "Could not send video frame: %s", netstrerror);
-            net_kill_strerror(netstrerror);
+            net_kill_strerror(av->mem, netstrerror);
             return TOXAV_ERR_SEND_FRAME_RTP_FAILED;
         }
     }
@@ -1507,7 +1509,7 @@ static bool call_prepare_transmission(ToxAVCall *call)
             goto FAILURE;
         }
 
-        call->audio_rtp = rtp_new(av->log, RTP_TYPE_AUDIO, av->tox, av, call->friend_number, call->bwc,
+        call->audio_rtp = rtp_new(av->log, av->mem, RTP_TYPE_AUDIO, av->tox, av, call->friend_number, call->bwc,
                                   call->audio, ac_queue_message);
 
         if (call->audio_rtp == nullptr) {
@@ -1523,7 +1525,7 @@ static bool call_prepare_transmission(ToxAVCall *call)
             goto FAILURE;
         }
 
-        call->video_rtp = rtp_new(av->log, RTP_TYPE_VIDEO, av->tox, av, call->friend_number, call->bwc,
+        call->video_rtp = rtp_new(av->log, av->mem, RTP_TYPE_VIDEO, av->tox, av, call->friend_number, call->bwc,
                                   call->video, vc_queue_message);
 
         if (call->video_rtp == nullptr) {

diff --git a/toxcore/BUILD.bazel b/toxcore/BUILD.bazel
@@ -192,6 +192,7 @@ cc_library(
     deps = [
         ":attributes",
         ":ccompat",
+        ":mem",
         "//c-toxcore/third_party:cmp",
     ],
 )
@@ -204,6 +205,7 @@ cc_test(
         ":bin_pack",
         ":bin_unpack",
         ":logger",
+        ":mem",
         "@com_google_googletest//:gtest",
         "@com_google_googletest//:gtest_main",
     ],

diff --git a/toxcore/Messenger.c b/toxcore/Messenger.c
@@ -3171,7 +3171,7 @@ static bool handle_groups_load(void *obj, Bin_Unpack *bu)
 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)) {
+    if (!bin_unpack_obj(m->mem, handle_groups_load, m, data, length)) {
         LOGGER_ERROR(m->log, "msgpack failed to unpack groupchats array");
         return STATE_LOAD_STATUS_ERROR;
     }