From c3a938e38c1e0c9c915f713bb6881c4f02d3823c Mon Sep 17 00:00:00 2001 From: iphydf Date: Sun, 13 Mar 2022 13:32:01 +0000 Subject: [PATCH] cleanup: Avoid `memset` on structs. Most of these were safe (some were not), but even the safe ones can become unsafe (non-portable) later on when adding pointer or float members. --- other/bootstrap_daemon/docker/tox-bootstrapd.sha256 | 2 +- testing/BUILD.bazel | 1 - toxcore/DHT.c | 6 ++++-- toxcore/DHT.h | 2 ++ toxcore/Messenger.c | 6 ++++-- toxcore/TCP_connection.c | 12 ++++++++---- toxcore/friend_connection.c | 8 +++++--- toxcore/group.c | 10 ++++++---- toxcore/network.c | 9 ++++++--- toxcore/network.h | 2 ++ toxcore/onion_client.c | 12 +++++------- 11 files changed, 43 insertions(+), 27 deletions(-) diff --git a/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 b/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 index dc72e8dd..f701caad 100644 --- a/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 +++ b/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 @@ -1 +1 @@ -6a7b0559334d281aacd5ebc18ed5523da1ac49d9dd09d81a33166a598d4a97b5 /usr/local/bin/tox-bootstrapd +d17e339f89331fa295f92bbe3c3a1dad6331d523ae9c898cab3d9b5cbc0e8ea9 /usr/local/bin/tox-bootstrapd diff --git a/testing/BUILD.bazel b/testing/BUILD.bazel index a53ba689..0bd5c4f5 100644 --- a/testing/BUILD.bazel +++ b/testing/BUILD.bazel @@ -16,7 +16,6 @@ sh_test( "-Wno-boolean-return", "-Wno-callback-names", "-Wno-enum-names", - "-Wno-memcpy-structs", "-Wno-type-check", "+RTS", "-N3", diff --git a/toxcore/DHT.c b/toxcore/DHT.c index e3f68d24..7d6b78ec 100644 --- a/toxcore/DHT.c +++ b/toxcore/DHT.c @@ -68,6 +68,9 @@ struct DHT_Friend { unsigned int num_to_bootstrap; }; +static const DHT_Friend empty_dht_friend = {{0}}; +const Node_format empty_node_format = {{0}}; + typedef struct Cryptopacket_Handler { cryptopacket_handler_cb *function; void *object; @@ -582,7 +585,6 @@ int unpack_ip_port(IP_Port *ip_port, const uint8_t *data, uint16_t length, bool return -1; } - const IP_Port empty_ip_port = {{{0}}}; *ip_port = empty_ip_port; if (is_ipv4) { @@ -1652,7 +1654,7 @@ int dht_addfriend(DHT *dht, const uint8_t *public_key, dht_ip_cb *ip_callback, dht->friends_list = temp; DHT_Friend *const dht_friend = &dht->friends_list[dht->num_friends]; - memset(dht_friend, 0, sizeof(DHT_Friend)); + *dht_friend = empty_dht_friend; memcpy(dht_friend->public_key, public_key, CRYPTO_PUBLIC_KEY_SIZE); dht_friend->nat.nat_ping_id = random_u64(); diff --git a/toxcore/DHT.h b/toxcore/DHT.h index 4a14c205..0c21693a 100644 --- a/toxcore/DHT.h +++ b/toxcore/DHT.h @@ -162,6 +162,8 @@ typedef struct Node_format { IP_Port ip_port; } Node_format; +extern const Node_format empty_node_format; + typedef struct DHT_Friend DHT_Friend; non_null() const uint8_t *dht_friend_public_key(const DHT_Friend *dht_friend); diff --git a/toxcore/Messenger.c b/toxcore/Messenger.c index f192c091..ce282d71 100644 --- a/toxcore/Messenger.c +++ b/toxcore/Messenger.c @@ -24,6 +24,8 @@ static_assert(MAX_CONCURRENT_FILE_PIPES <= UINT8_MAX + 1, "uint8_t cannot represent all file transfer numbers"); +static const Friend empty_friend = {{0}}; + /** @brief Set the size of the friend list to numfriends. * * @retval -1 if realloc fails. @@ -163,7 +165,7 @@ static int32_t init_new_friend(Messenger *m, const uint8_t *real_pk, uint8_t sta return FAERR_NOMEM; } - memset(&m->friendlist[m->numfriends], 0, sizeof(Friend)); + m->friendlist[m->numfriends] = empty_friend; const int friendcon_id = new_friend_connection(m->fr_c, real_pk); @@ -418,7 +420,7 @@ int m_delfriend(Messenger *m, int32_t friendnumber) } kill_friend_connection(m->fr_c, m->friendlist[friendnumber].friendcon_id); - memset(&m->friendlist[friendnumber], 0, sizeof(Friend)); + m->friendlist[friendnumber] = empty_friend; uint32_t i; diff --git a/toxcore/TCP_connection.c b/toxcore/TCP_connection.c index 1e6800b8..32563df7 100644 --- a/toxcore/TCP_connection.c +++ b/toxcore/TCP_connection.c @@ -47,6 +47,10 @@ struct TCP_Connections { }; +static const TCP_Connection_to empty_tcp_connection_to = {0}; +static const TCP_con empty_tcp_con = {0}; + + const uint8_t *tcp_connections_public_key(const TCP_Connections *tcp_c) { return tcp_c->self_public_key; @@ -158,7 +162,7 @@ static int create_connection(TCP_Connections *tcp_c) if (realloc_TCP_Connection_to(&tcp_c->connections, tcp_c->connections_length + 1) == 0) { id = tcp_c->connections_length; ++tcp_c->connections_length; - memset(&tcp_c->connections[id], 0, sizeof(TCP_Connection_to)); + tcp_c->connections[id] = empty_tcp_connection_to; } return id; @@ -183,7 +187,7 @@ static int create_tcp_connection(TCP_Connections *tcp_c) if (realloc_TCP_con(&tcp_c->tcp_connections, tcp_c->tcp_connections_length + 1) == 0) { id = tcp_c->tcp_connections_length; ++tcp_c->tcp_connections_length; - memset(&tcp_c->tcp_connections[id], 0, sizeof(TCP_con)); + tcp_c->tcp_connections[id] = empty_tcp_con; } return id; @@ -202,7 +206,7 @@ static int wipe_connection(TCP_Connections *tcp_c, int connections_number) } uint32_t i; - memset(&tcp_c->connections[connections_number], 0, sizeof(TCP_Connection_to)); + tcp_c->connections[connections_number] = empty_tcp_connection_to; for (i = tcp_c->connections_length; i != 0; --i) { if (tcp_c->connections[i - 1].status != TCP_CONN_NONE) { @@ -230,7 +234,7 @@ static int wipe_tcp_connection(TCP_Connections *tcp_c, int tcp_connections_numbe return -1; } - memset(&tcp_c->tcp_connections[tcp_connections_number], 0, sizeof(TCP_con)); + tcp_c->tcp_connections[tcp_connections_number] = empty_tcp_con; uint32_t i; diff --git a/toxcore/friend_connection.c b/toxcore/friend_connection.c index 5e8d2bff..dd63d335 100644 --- a/toxcore/friend_connection.c +++ b/toxcore/friend_connection.c @@ -54,6 +54,8 @@ struct Friend_Conn { bool hosting_tcp_relay; }; +static const Friend_Conn empty_friend_conn = {0}; + struct Friend_Connections { const Mono_Time *mono_time; @@ -151,7 +153,7 @@ static int create_friend_conn(Friend_Connections *fr_c) const int id = fr_c->num_cons; ++fr_c->num_cons; - memset(&fr_c->conns[id], 0, sizeof(Friend_Conn)); + fr_c->conns[id] = empty_friend_conn; return id; } @@ -168,7 +170,7 @@ static int wipe_friend_conn(Friend_Connections *fr_c, int friendcon_id) return -1; } - memset(&fr_c->conns[friendcon_id], 0, sizeof(Friend_Conn)); + fr_c->conns[friendcon_id] = empty_friend_conn; uint32_t i; @@ -245,7 +247,7 @@ static int friend_add_tcp_relay(Friend_Connections *fr_c, int friendcon_id, cons for (unsigned i = 0; i < FRIEND_MAX_STORED_TCP_RELAYS; ++i) { if (!net_family_is_unspec(friend_con->tcp_relays[i].ip_port.ip.family) && pk_equal(friend_con->tcp_relays[i].public_key, public_key)) { - memset(&friend_con->tcp_relays[i], 0, sizeof(Node_format)); + friend_con->tcp_relays[i] = empty_node_format; } } diff --git a/toxcore/group.c b/toxcore/group.c index b1a9203d..e135ec90 100644 --- a/toxcore/group.c +++ b/toxcore/group.c @@ -156,6 +156,9 @@ struct Group_Chats { lossy_packet_cb *lossy_packethandlers[256]; }; +static const Group_c empty_group_c = {0}; +static const Group_Peer empty_group_peer = {{0}}; + /** * Packet type IDs as per the protocol specification. */ @@ -260,8 +263,7 @@ static bool realloc_conferences(Group_Chats *g_c, uint16_t num) non_null() static void setup_conference(Group_c *g) { - memset(g, 0, sizeof(Group_c)); - + *g = empty_group_c; g->maxfrozen = MAX_FROZEN_DEFAULT; } @@ -833,7 +835,7 @@ static int addpeer(Group_Chats *g_c, uint32_t groupnumber, const uint8_t *real_p return -1; } - memset(&temp[g->numpeers], 0, sizeof(Group_Peer)); + temp[g->numpeers] = empty_group_peer; g->group = temp; const uint32_t new_index = g->numpeers; @@ -3617,7 +3619,7 @@ static uint32_t load_group(Group_c *g, const Group_Chats *g_c, const uint8_t *da g->frozen = tmp_frozen; Group_Peer *peer = &g->frozen[j]; - memset(peer, 0, sizeof(Group_Peer)); + *peer = empty_group_peer; pk_copy(peer->real_pk, data); data += CRYPTO_PUBLIC_KEY_SIZE; diff --git a/toxcore/network.c b/toxcore/network.c index 316dd211..7ed81a01 100644 --- a/toxcore/network.c +++ b/toxcore/network.c @@ -340,6 +340,9 @@ static void fill_addr6(const IP6 *ip, struct in6_addr *addr) #define INADDR_LOOPBACK 0x7f000001 #endif +static const IP empty_ip = {{0}}; +const IP_Port empty_ip_port = {{{0}}}; + const IP4 ip4_broadcast = { INADDR_BROADCAST }; const IP6 ip6_broadcast = { { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff } @@ -1172,7 +1175,7 @@ void ip_reset(IP *ip) return; } - memset(ip, 0, sizeof(IP)); + *ip = empty_ip; } /** nulls out ip_port */ @@ -1182,7 +1185,7 @@ void ipport_reset(IP_Port *ipport) return; } - memset(ipport, 0, sizeof(IP_Port)); + *ipport = empty_ip_port; } /** nulls out ip, sets family according to flag */ @@ -1192,7 +1195,7 @@ void ip_init(IP *ip, bool ipv6enabled) return; } - memset(ip, 0, sizeof(IP)); + *ip = empty_ip; ip->family = ipv6enabled ? net_family_ipv6 : net_family_ipv4; } diff --git a/toxcore/network.h b/toxcore/network.h index f7860cf1..7b59f1a8 100644 --- a/toxcore/network.h +++ b/toxcore/network.h @@ -140,6 +140,8 @@ typedef struct IP_Port { uint16_t port; } IP_Port; +extern const IP_Port empty_ip_port; + typedef struct Socket { int sock; } Socket; diff --git a/toxcore/onion_client.c b/toxcore/onion_client.c index 3fb05682..9ea86153 100644 --- a/toxcore/onion_client.c +++ b/toxcore/onion_client.c @@ -90,6 +90,8 @@ typedef struct Onion_Friend { uint32_t dht_pk_callback_number; } Onion_Friend; +static const Onion_Friend empty_onion_friend = {false}; + typedef struct Onion_Data_Handler { oniondata_handler_cb *function; void *object; @@ -287,9 +289,7 @@ static uint16_t random_nodes_path_onion(const Onion_Client *onion_c, Node_format } if (num_nodes >= 2) { - nodes[0] = (Node_format) { - 0 - }; + nodes[0] = empty_node_format; nodes[0].ip_port.ip.family = net_family_tcp_family; nodes[0].ip_port.ip.ip.v4.uint32 = random_tcp; @@ -304,9 +304,7 @@ static uint16_t random_nodes_path_onion(const Onion_Client *onion_c, Node_format return 0; } - nodes[0] = (Node_format) { - 0 - }; + nodes[0] = empty_node_format; nodes[0].ip_port.ip.family = net_family_tcp_family; nodes[0].ip_port.ip.ip.v4.uint32 = random_tcp; @@ -1341,7 +1339,7 @@ int onion_addfriend(Onion_Client *onion_c, const uint8_t *public_key) } index = onion_c->num_friends; - memset(&onion_c->friends_list[onion_c->num_friends], 0, sizeof(Onion_Friend)); + onion_c->friends_list[onion_c->num_friends] = empty_onion_friend; ++onion_c->num_friends; }