Skip to content

Commit

Permalink
cleanup: Remove all uses of SIZEOF_VLA.
Browse files Browse the repository at this point in the history
This is step 1 towards removing VLAs altogether.
  • Loading branch information
iphydf committed Jan 25, 2024
1 parent 662c214 commit 5c093c4
Show file tree
Hide file tree
Showing 20 changed files with 135 additions and 107 deletions.
7 changes: 4 additions & 3 deletions auto_tests/TCP_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -266,13 +266,14 @@ static void kill_tcp_con(struct sec_TCP_con *con)
static int write_packet_tcp_test_connection(const Logger *logger, struct sec_TCP_con *con, const uint8_t *data,
uint16_t length)
{
VLA(uint8_t, packet, sizeof(uint16_t) + length + CRYPTO_MAC_SIZE);
const uint16_t packet_size = sizeof(uint16_t) + length + CRYPTO_MAC_SIZE;
VLA(uint8_t, packet, packet_size);

uint16_t c_length = net_htons(length + CRYPTO_MAC_SIZE);
memcpy(packet, &c_length, sizeof(uint16_t));
int len = encrypt_data_symmetric(con->shared_key, con->sent_nonce, data, length, packet + sizeof(uint16_t));

if ((unsigned int)len != (SIZEOF_VLA(packet) - sizeof(uint16_t))) {
if ((unsigned int)len != (packet_size - sizeof(uint16_t))) {
return -1;
}

Expand All @@ -282,7 +283,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, SIZEOF_VLA(packet), &localhost) == SIZEOF_VLA(packet),
ck_assert_msg(net_send(con->ns, logger, con->sock, packet, packet_size, &localhost) == packet_size,
"Failed to send a packet.");
return 0;
}
Expand Down
5 changes: 2 additions & 3 deletions auto_tests/save_friend_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,12 @@ struct test_data {
static void set_random(Tox *m, const Random *rng, bool (*setter)(Tox *, const uint8_t *, size_t, Tox_Err_Set_Info *), size_t length)
{
VLA(uint8_t, text, length);
uint32_t i;

for (i = 0; i < length; ++i) {
for (uint32_t i = 0; i < length; ++i) {
text[i] = random_u08(rng);
}

setter(m, text, SIZEOF_VLA(text), nullptr);
setter(m, text, length, nullptr);
}

static void alloc_string(uint8_t **to, size_t length)
Expand Down
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 @@
c028527fe5eecde7daa6ac9dacc47bd4bb765463e8e4b5af3881789797bf81a8 /usr/local/bin/tox-bootstrapd
8a2cc2e20ac0688b42c6e6211cb1bc5797c276ed52a2322d8b786f8ad562d535 /usr/local/bin/tox-bootstrapd
5 changes: 0 additions & 5 deletions other/docker/misra/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -126,11 +126,6 @@ SUPPRESSIONS += 19.2
#
# Reason: We believe it should be used when #define is used in block scope.
SUPPRESSIONS += 20.5
# The # and ## preprocessor operators should not be used.
#
# TODO(iphydf): Remove suppression when VLAs are gone. This is only used in
# the SIZEOF_VLA macro.
SUPPRESSIONS += 20.10
# #define and #undef shall not be used on a reserved identifier or reserved macro name.
#
# Reason: Needed for feature test macros like _DEFAULT_SOURCE.
Expand Down
9 changes: 5 additions & 4 deletions toxav/rtp.c
Original file line number Diff line number Diff line change
Expand Up @@ -806,8 +806,9 @@ int rtp_send_data(RTPSession *session, const uint8_t *data, uint32_t length,
header.flags |= RTP_KEY_FRAME;
}

VLA(uint8_t, rdata, length + RTP_HEADER_SIZE + 1);
memset(rdata, 0, SIZEOF_VLA(rdata));
const uint16_t rdata_size = length + RTP_HEADER_SIZE + 1;
VLA(uint8_t, rdata, rdata_size);
memset(rdata, 0, rdata_size);
rdata[0] = session->payload_type; // packet id == payload_type

if (MAX_CRYPTO_DATA_SIZE > (length + RTP_HEADER_SIZE + 1)) {
Expand All @@ -818,10 +819,10 @@ int rtp_send_data(RTPSession *session, const uint8_t *data, uint32_t length,
rtp_header_pack(rdata + 1, &header);
memcpy(rdata + 1 + RTP_HEADER_SIZE, data, length);

if (-1 == rtp_send_custom_lossy_packet(session->tox, session->friend_number, rdata, SIZEOF_VLA(rdata))) {
if (-1 == rtp_send_custom_lossy_packet(session->tox, session->friend_number, rdata, rdata_size)) {
char *netstrerror = net_new_strerror(net_error());
LOGGER_WARNING(session->m->log, "RTP send failed (len: %u)! net error: %s",
(unsigned)SIZEOF_VLA(rdata), netstrerror);
rdata_size, netstrerror);

Check warning on line 825 in toxav/rtp.c

View check run for this annotation

Codecov / codecov/patch

toxav/rtp.c#L825

Added line #L825 was not covered by tests
net_kill_strerror(netstrerror);
}
} else {
Expand Down
6 changes: 4 additions & 2 deletions toxav/toxav.c
Original file line number Diff line number Diff line change
Expand Up @@ -850,12 +850,14 @@ bool toxav_audio_send_frame(ToxAV *av, uint32_t friend_number, const int16_t *pc
goto RETURN;
}

VLA(uint8_t, dest, sample_count + sizeof(sampling_rate)); /* This is more than enough always */
/* This is more than enough always */
const uint16_t dest_size = sample_count + sizeof(sampling_rate);
VLA(uint8_t, dest, dest_size);

sampling_rate = net_htonl(sampling_rate);
memcpy(dest, &sampling_rate, sizeof(sampling_rate));
const int vrc = opus_encode(call->audio->encoder, pcm, sample_count,
dest + sizeof(sampling_rate), SIZEOF_VLA(dest) - sizeof(sampling_rate));
dest + sizeof(sampling_rate), dest_size - sizeof(sampling_rate));

Check warning on line 860 in toxav/toxav.c

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

toxav/toxav.c#L860

MISRA 18.4 rule

if (vrc < 0) {
LOGGER_WARNING(av->m->log, "Failed to encode frame %s", opus_strerror(vrc));
Expand Down
12 changes: 7 additions & 5 deletions toxcore/DHT.c
Original file line number Diff line number Diff line change
Expand Up @@ -1487,13 +1487,14 @@ static int sendnodes_ipv6(const DHT *dht, const IP_Port *ip_port, const uint8_t
memcpy(plain + 1 + nodes_length, sendback_data, length);

const uint32_t crypto_size = 1 + CRYPTO_PUBLIC_KEY_SIZE + CRYPTO_NONCE_SIZE + CRYPTO_MAC_SIZE;
VLA(uint8_t, data, 1 + nodes_length + length + crypto_size);
const uint32_t data_size = 1 + nodes_length + length + crypto_size;
VLA(uint8_t, data, data_size);

const int len = dht_create_packet(dht->mem, dht->rng,
dht->self_public_key, shared_encryption_key, NET_PACKET_SEND_NODES_IPV6,
plain, 1 + nodes_length + length, data, SIZEOF_VLA(data));
plain, 1 + nodes_length + length, data, data_size);

if (len != SIZEOF_VLA(data)) {
if (len != data_size) {
return -1;
}

Expand Down Expand Up @@ -1576,7 +1577,8 @@ static bool handle_sendnodes_core(void *object, const IP_Port *source, const uin
return false;
}

VLA(uint8_t, plain, 1 + data_size + sizeof(uint64_t));
const uint32_t plain_size = 1 + data_size + sizeof(uint64_t);
VLA(uint8_t, plain, plain_size);
const uint8_t *shared_key = dht_get_shared_key_sent(dht, packet + 1);
const int len = decrypt_data_symmetric(
shared_key,
Expand All @@ -1585,7 +1587,7 @@ static bool handle_sendnodes_core(void *object, const IP_Port *source, const uin
1 + data_size + sizeof(uint64_t) + CRYPTO_MAC_SIZE,
plain);

if ((unsigned int)len != SIZEOF_VLA(plain)) {
if ((uint32_t)len != plain_size) {
return false;
}

Expand Down
15 changes: 9 additions & 6 deletions toxcore/Messenger.c
Original file line number Diff line number Diff line change
Expand Up @@ -1223,7 +1223,8 @@ static bool file_sendrequest(const Messenger *m, int32_t friendnumber, uint8_t f
return false;
}

VLA(uint8_t, packet, 1 + sizeof(file_type) + sizeof(filesize) + FILE_ID_LENGTH + filename_length);
const uint16_t packet_size = 1 + sizeof(file_type) + sizeof(filesize) + FILE_ID_LENGTH + filename_length;
VLA(uint8_t, packet, packet_size);
packet[0] = filenumber;
file_type = net_htonl(file_type);
memcpy(packet + 1, &file_type, sizeof(file_type));
Expand All @@ -1234,7 +1235,7 @@ static bool file_sendrequest(const Messenger *m, int32_t friendnumber, uint8_t f
memcpy(packet + 1 + sizeof(file_type) + sizeof(filesize) + FILE_ID_LENGTH, filename, filename_length);
}

return write_cryptpacket_id(m, friendnumber, PACKET_ID_FILE_SENDREQUEST, packet, SIZEOF_VLA(packet), false);
return write_cryptpacket_id(m, friendnumber, PACKET_ID_FILE_SENDREQUEST, packet, packet_size, false);
}

/** @brief Send a file send request.
Expand Down Expand Up @@ -1301,7 +1302,8 @@ static bool send_file_control_packet(const Messenger *m, int32_t friendnumber, b
return false;
}

VLA(uint8_t, packet, 3 + data_length);
const uint16_t packet_size = 3 + data_length;
VLA(uint8_t, packet, packet_size);

packet[0] = inbound ? 1 : 0;
packet[1] = filenumber;
Expand All @@ -1311,7 +1313,7 @@ static bool send_file_control_packet(const Messenger *m, int32_t friendnumber, b
memcpy(packet + 3, data, data_length);
}

return write_cryptpacket_id(m, friendnumber, PACKET_ID_FILE_CONTROL, packet, SIZEOF_VLA(packet), false);
return write_cryptpacket_id(m, friendnumber, PACKET_ID_FILE_CONTROL, packet, packet_size, false);
}

/** @brief Send a file control request.
Expand Down Expand Up @@ -1501,7 +1503,8 @@ static int64_t send_file_data_packet(const Messenger *m, int32_t friendnumber, u
return -1;
}

VLA(uint8_t, packet, 2 + length);
const uint16_t packet_size = 2 + length;
VLA(uint8_t, packet, packet_size);
packet[0] = PACKET_ID_FILE_DATA;
packet[1] = filenumber;

Expand All @@ -1510,7 +1513,7 @@ static int64_t send_file_data_packet(const Messenger *m, int32_t friendnumber, u
}

return write_cryptpacket(m->net_crypto, friend_connection_crypt_connection_id(m->fr_c,
m->friendlist[friendnumber].friendcon_id), packet, SIZEOF_VLA(packet), true);
m->friendlist[friendnumber].friendcon_id), packet, packet_size, true);
}

#define MAX_FILE_DATA_SIZE (MAX_CRYPTO_DATA_SIZE - 2)
Expand Down
15 changes: 9 additions & 6 deletions toxcore/TCP_client.c
Original file line number Diff line number Diff line change
Expand Up @@ -394,10 +394,11 @@ int send_data(const Logger *logger, TCP_Client_Connection *con, uint8_t con_id,
return 0;
}

VLA(uint8_t, packet, 1 + length);
const uint16_t packet_size = 1 + length;
VLA(uint8_t, packet, packet_size);
packet[0] = con_id + NUM_RESERVED_PORTS;
memcpy(packet + 1, data, length);
return write_packet_tcp_secure_connection(logger, &con->con, packet, SIZEOF_VLA(packet), false);
return write_packet_tcp_secure_connection(logger, &con->con, packet, packet_size, false);
}

/**
Expand All @@ -412,11 +413,12 @@ int send_oob_packet(const Logger *logger, TCP_Client_Connection *con, const uint
return -1;
}

VLA(uint8_t, packet, 1 + CRYPTO_PUBLIC_KEY_SIZE + length);
const uint16_t packet_size = 1 + CRYPTO_PUBLIC_KEY_SIZE + length;
VLA(uint8_t, packet, packet_size);
packet[0] = TCP_PACKET_OOB_SEND;
memcpy(packet + 1, public_key, CRYPTO_PUBLIC_KEY_SIZE);
memcpy(packet + 1 + CRYPTO_PUBLIC_KEY_SIZE, data, length);
return write_packet_tcp_secure_connection(logger, &con->con, packet, SIZEOF_VLA(packet), false);
return write_packet_tcp_secure_connection(logger, &con->con, packet, packet_size, false);
}


Expand Down Expand Up @@ -536,10 +538,11 @@ int send_disconnect_request(const Logger *logger, TCP_Client_Connection *con, ui
*/
int send_onion_request(const Logger *logger, TCP_Client_Connection *con, const uint8_t *data, uint16_t length)
{
VLA(uint8_t, packet, 1 + length);
const uint16_t packet_size = 1 + length;
VLA(uint8_t, packet, packet_size);
packet[0] = TCP_PACKET_ONION_REQUEST;
memcpy(packet + 1, data, length);
return write_packet_tcp_secure_connection(logger, &con->con, packet, SIZEOF_VLA(packet), false);
return write_packet_tcp_secure_connection(logger, &con->con, packet, packet_size, false);
}

void onion_response_handler(TCP_Client_Connection *con, tcp_onion_response_cb *onion_callback, void *object)
Expand Down
19 changes: 10 additions & 9 deletions toxcore/TCP_common.c
Original file line number Diff line number Diff line change
Expand Up @@ -151,46 +151,47 @@ int write_packet_tcp_secure_connection(const Logger *logger, TCP_Connection *con
}
}

VLA(uint8_t, packet, sizeof(uint16_t) + length + CRYPTO_MAC_SIZE);
const uint16_t packet_size = sizeof(uint16_t) + length + CRYPTO_MAC_SIZE;
VLA(uint8_t, packet, packet_size);

uint16_t c_length = net_htons(length + CRYPTO_MAC_SIZE);
memcpy(packet, &c_length, sizeof(uint16_t));
int len = encrypt_data_symmetric(con->shared_key, con->sent_nonce, data, length, packet + sizeof(uint16_t));

if ((unsigned int)len != (SIZEOF_VLA(packet) - sizeof(uint16_t))) {
if ((unsigned int)len != (packet_size - sizeof(uint16_t))) {
return -1;
}

if (priority) {
len = sendpriority ? net_send(con->ns, logger, con->sock, packet, SIZEOF_VLA(packet), &con->ip_port) : 0;
len = sendpriority ? net_send(con->ns, logger, con->sock, packet, packet_size, &con->ip_port) : 0;

if (len <= 0) {
len = 0;
}

increment_nonce(con->sent_nonce);

if ((unsigned int)len == SIZEOF_VLA(packet)) {
if ((unsigned int)len == packet_size) {
return 1;
}

return add_priority(con, packet, SIZEOF_VLA(packet), len) ? 1 : 0;
return add_priority(con, packet, packet_size, len) ? 1 : 0;
}

len = net_send(con->ns, logger, con->sock, packet, SIZEOF_VLA(packet), &con->ip_port);
len = net_send(con->ns, logger, con->sock, packet, packet_size, &con->ip_port);

if (len <= 0) {
return 0;
}

increment_nonce(con->sent_nonce);

if ((unsigned int)len == SIZEOF_VLA(packet)) {
if ((unsigned int)len == packet_size) {
return 1;
}

memcpy(con->last_packet, packet, SIZEOF_VLA(packet));
con->last_packet_length = SIZEOF_VLA(packet);
memcpy(con->last_packet, packet, packet_size);
con->last_packet_length = packet_size;

Check warning on line 194 in toxcore/TCP_common.c

View check run for this annotation

Codecov / codecov/patch

toxcore/TCP_common.c#L193-L194

Added lines #L193 - L194 were not covered by tests
con->last_packet_sent = len;
return 1;
}
Expand Down
15 changes: 9 additions & 6 deletions toxcore/TCP_server.c
Original file line number Diff line number Diff line change
Expand Up @@ -532,12 +532,13 @@ static int handle_tcp_oob_send(TCP_Server *tcp_server, uint32_t con_id, const ui
const int other_index = get_tcp_connection_index(tcp_server, public_key);

if (other_index != -1) {
VLA(uint8_t, resp_packet, 1 + CRYPTO_PUBLIC_KEY_SIZE + length);
const uint16_t resp_packet_size = 1 + CRYPTO_PUBLIC_KEY_SIZE + length;
VLA(uint8_t, resp_packet, resp_packet_size);
resp_packet[0] = TCP_PACKET_OOB_RECV;
memcpy(resp_packet + 1, con->public_key, CRYPTO_PUBLIC_KEY_SIZE);
memcpy(resp_packet + 1 + CRYPTO_PUBLIC_KEY_SIZE, data, length);
write_packet_tcp_secure_connection(tcp_server->logger, &tcp_server->accepted_connection_array[other_index].con,
resp_packet, SIZEOF_VLA(resp_packet), false);
resp_packet, resp_packet_size, false);
}

return 0;
Expand Down Expand Up @@ -620,11 +621,12 @@ static int handle_onion_recv_1(void *object, const IP_Port *dest, const uint8_t

TCP_Secure_Connection *con = &tcp_server->accepted_connection_array[index];

VLA(uint8_t, packet, 1 + length);
const uint16_t packet_size = 1 + length;
VLA(uint8_t, packet, packet_size);
memcpy(packet + 1, data, length);
packet[0] = TCP_PACKET_ONION_RESPONSE;

if (write_packet_tcp_secure_connection(tcp_server->logger, &con->con, packet, SIZEOF_VLA(packet), false) != 1) {
if (write_packet_tcp_secure_connection(tcp_server->logger, &con->con, packet, packet_size, false) != 1) {
return 1;
}

Expand Down Expand Up @@ -660,11 +662,12 @@ static bool handle_forward_reply_tcp(void *object, const uint8_t *sendback_data,
return false;
}

VLA(uint8_t, packet, 1 + length);
const uint16_t packet_size = 1 + length;
VLA(uint8_t, packet, packet_size);
memcpy(packet + 1, data, length);
packet[0] = TCP_PACKET_FORWARDING;

return write_packet_tcp_secure_connection(tcp_server->logger, &con->con, packet, SIZEOF_VLA(packet), false) == 1;
return write_packet_tcp_secure_connection(tcp_server->logger, &con->con, packet, packet_size, false) == 1;
}

/**
Expand Down
5 changes: 1 addition & 4 deletions toxcore/ccompat.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
#if !defined(DISABLE_VLA) && !defined(_MSC_VER) && defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
// C99 VLAs.
#define ALLOC_VLA(type, name, size) type name[size]
#define SIZEOF_VLA sizeof
#else

// Emulation using alloca.
Expand All @@ -47,9 +46,7 @@
#endif

#define ALLOC_VLA(type, name, size) \
const size_t name##_vla_size = (size) * sizeof(type); \
type *const name = (type *)alloca(name##_vla_size)
#define SIZEOF_VLA(name) name##_vla_size
type *const name = (type *)alloca((size) * sizeof(type))

#endif

Expand Down
7 changes: 4 additions & 3 deletions toxcore/friend_connection.c
Original file line number Diff line number Diff line change
Expand Up @@ -886,18 +886,19 @@ int send_friend_request_packet(Friend_Connections *fr_c, int friendcon_id, uint3
return -1;
}

VLA(uint8_t, packet, 1 + sizeof(nospam_num) + length);
const uint16_t packet_size = 1 + sizeof(nospam_num) + length;
VLA(uint8_t, packet, packet_size);
memcpy(packet + 1, &nospam_num, sizeof(nospam_num));
memcpy(packet + 1 + sizeof(nospam_num), data, length);

if (friend_con->status == FRIENDCONN_STATUS_CONNECTED) {
packet[0] = PACKET_ID_FRIEND_REQUESTS;
return write_cryptpacket(fr_c->net_crypto, friend_con->crypt_connection_id, packet, SIZEOF_VLA(packet),
return write_cryptpacket(fr_c->net_crypto, friend_con->crypt_connection_id, packet, packet_size,

Check warning on line 896 in toxcore/friend_connection.c

View check run for this annotation

Codecov / codecov/patch

toxcore/friend_connection.c#L896

Added line #L896 was not covered by tests
false) != -1 ? 1 : 0;
}

packet[0] = CRYPTO_PACKET_FRIEND_REQ;
const int num = send_onion_data(fr_c->onion_c, friend_con->onion_friendnum, packet, SIZEOF_VLA(packet));
const int num = send_onion_data(fr_c->onion_c, friend_con->onion_friendnum, packet, packet_size);

if (num <= 0) {
return -1;
Expand Down
4 changes: 2 additions & 2 deletions toxcore/friend_requests.c
Original file line number Diff line number Diff line change
Expand Up @@ -148,10 +148,10 @@ static int friendreq_handlepacket(void *object, const uint8_t *source_pubkey, co

addto_receivedlist(fr, source_pubkey);

const uint32_t message_len = length - sizeof(fr->nospam);
const uint16_t message_len = length - sizeof(fr->nospam);
VLA(uint8_t, message, message_len + 1);
memcpy(message, data + sizeof(fr->nospam), message_len);
message[SIZEOF_VLA(message) - 1] = 0; /* Be sure the message is null terminated. */
message[message_len] = 0; /* Be sure the message is null terminated. TODO(iphydf): But why? */

fr->handle_friendrequest(fr->handle_friendrequest_object, source_pubkey, message, message_len, userdata);
return 0;
Expand Down
Loading

0 comments on commit 5c093c4

Please sign in to comment.