Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: packet broadcast functions now return errors #2542

Merged
merged 1 commit into from
Jan 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@
7e86d4f1c4aadce01a03153f2101ac1486f6de65f824b7b0cccbbfbf832c180a /usr/local/bin/tox-bootstrapd
423687bc6adc323174a2ab4e2ee8443e6c21c4d6509942af469b4bc16db6bfa4 /usr/local/bin/tox-bootstrapd
70 changes: 47 additions & 23 deletions toxcore/group_chats.c
Original file line number Diff line number Diff line change
Expand Up @@ -2262,34 +2262,56 @@
return ret;
}

/** @brief Sends a lossless packet of type and length to all confirmed peers. */
/** @brief Sends a lossless packet of type and length to all confirmed peers.
*
* Return true if packet is successfully sent to at least one peer.
*/
non_null()
static void send_gc_lossless_packet_all_peers(const GC_Chat *chat, const uint8_t *data, uint16_t length, uint8_t type)
static bool send_gc_lossless_packet_all_peers(const GC_Chat *chat, const uint8_t *data, uint16_t length, uint8_t type)
{
uint32_t sent = 0;

for (uint32_t i = 1; i < chat->numpeers; ++i) {
GC_Connection *gconn = get_gc_connection(chat, i);

assert(gconn != nullptr);

if (gconn->confirmed) {
send_lossless_group_packet(chat, gconn, data, length, type);
if (!gconn->confirmed) {
continue;
}

if (send_lossless_group_packet(chat, gconn, data, length, type)) {
++sent;
}
}

return sent > 0 || chat->numpeers <= 1;
}

/** @brief Sends a lossy packet of type and length to all confirmed peers. */
/** @brief Sends a lossy packet of type and length to all confirmed peers.
*
* Return true if packet is successfully sent to at least one peer.
*/
non_null()
static void send_gc_lossy_packet_all_peers(const GC_Chat *chat, const uint8_t *data, uint16_t length, uint8_t type)
static bool send_gc_lossy_packet_all_peers(const GC_Chat *chat, const uint8_t *data, uint16_t length, uint8_t type)
{
uint32_t sent = 0;

for (uint32_t i = 1; i < chat->numpeers; ++i) {
const GC_Connection *gconn = get_gc_connection(chat, i);

assert(gconn != nullptr);

if (gconn->confirmed) {
send_lossy_group_packet(chat, gconn, data, length, type);
if (!gconn->confirmed) {
continue;

Check warning on line 2306 in toxcore/group_chats.c

View check run for this annotation

Codecov / codecov/patch

toxcore/group_chats.c#L2306

Added line #L2306 was not covered by tests
}

if (send_lossy_group_packet(chat, gconn, data, length, type)) {
++sent;
}
}

return sent > 0 || chat->numpeers <= 1;
}

/** @brief Creates packet with broadcast header info followed by data of length.
Expand Down Expand Up @@ -2329,11 +2351,11 @@

const uint16_t packet_len = make_gc_broadcast_header(data, length, packet, bc_type);

send_gc_lossless_packet_all_peers(chat, packet, packet_len, GP_BROADCAST);
const bool ret = send_gc_lossless_packet_all_peers(chat, packet, packet_len, GP_BROADCAST);

free(packet);

return true;
return ret;
}

non_null()
Expand Down Expand Up @@ -2787,9 +2809,7 @@
return false;
}

send_gc_lossless_packet_all_peers(chat, packet, (uint16_t)packet_len, GP_SHARED_STATE);

return true;
return send_gc_lossless_packet_all_peers(chat, packet, (uint16_t)packet_len, GP_SHARED_STATE);
}

/** @brief Helper function for `do_gc_shared_state_changes()`.
Expand Down Expand Up @@ -3310,11 +3330,11 @@
return false;
}

send_gc_lossless_packet_all_peers(chat, packet, (uint16_t)packet_len, GP_SANCTIONS_LIST);
const bool ret = send_gc_lossless_packet_all_peers(chat, packet, (uint16_t)packet_len, GP_SANCTIONS_LIST);

Check warning on line 3333 in toxcore/group_chats.c

View check run for this annotation

Codecov / codecov/patch

toxcore/group_chats.c#L3333

Added line #L3333 was not covered by tests

free(packet);

return true;
return ret;

Check warning on line 3337 in toxcore/group_chats.c

View check run for this annotation

Codecov / codecov/patch

toxcore/group_chats.c#L3337

Added line #L3337 was not covered by tests
}

/** @brief Re-signs all sanctions list entries signed by public_sig_key and broadcasts
Expand Down Expand Up @@ -3356,11 +3376,11 @@
return false;
}

send_gc_lossless_packet_all_peers(chat, packet, length, GP_MOD_LIST);
const bool ret = send_gc_lossless_packet_all_peers(chat, packet, length, GP_MOD_LIST);

Check warning on line 3379 in toxcore/group_chats.c

View check run for this annotation

Codecov / codecov/patch

toxcore/group_chats.c#L3379

Added line #L3379 was not covered by tests

free(packet);

return true;
return ret;

Check warning on line 3383 in toxcore/group_chats.c

View check run for this annotation

Codecov / codecov/patch

toxcore/group_chats.c#L3383

Added line #L3383 was not covered by tests
}

/** @brief Sends a parting signal to the group.
Expand Down Expand Up @@ -3717,11 +3737,11 @@
return false;
}

send_gc_lossless_packet_all_peers(chat, packet, packet_buf_size, GP_TOPIC);
const bool ret = send_gc_lossless_packet_all_peers(chat, packet, packet_buf_size, GP_TOPIC);

free(packet);

return true;
return ret;
}

int gc_set_topic(GC_Chat *chat, const uint8_t *topic, uint16_t length)
Expand Down Expand Up @@ -5072,13 +5092,15 @@
return -3;
}

bool success;

if (lossless) {
send_gc_lossless_packet_all_peers(chat, data, length, GP_CUSTOM_PACKET);
success = send_gc_lossless_packet_all_peers(chat, data, length, GP_CUSTOM_PACKET);
} else {
send_gc_lossy_packet_all_peers(chat, data, length, GP_CUSTOM_PACKET);
success = send_gc_lossy_packet_all_peers(chat, data, length, GP_CUSTOM_PACKET);
}

return 0;
return success ? 0 : -4;
JFreegman marked this conversation as resolved.
Show resolved Hide resolved
}

/** @brief Handles a custom packet.
Expand Down Expand Up @@ -7729,7 +7751,9 @@

chat->connection_state = CS_DISCONNECTED;

send_gc_broadcast_message(chat, nullptr, 0, GM_PEER_EXIT);
if (!send_gc_broadcast_message(chat, nullptr, 0, GM_PEER_EXIT)) {
LOGGER_DEBUG(chat->log, "Failed to broadcast group exit packet");

Check warning on line 7755 in toxcore/group_chats.c

View check run for this annotation

Codecov / codecov/patch

toxcore/group_chats.c#L7755

Added line #L7755 was not covered by tests
}

for (uint32_t i = 1; i < chat->numpeers; ++i) {
GC_Connection *gconn = get_gc_connection(chat, i);
Expand Down
1 change: 1 addition & 0 deletions toxcore/group_chats.h
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ int gc_send_private_message(const GC_Chat *chat, uint32_t peer_id, uint8_t type,
* Returns -1 if the message is too long.
* Returns -2 if the message pointer is NULL or length is zero.
* Returns -3 if the sender has the observer role.
* Returns -4 if the packet did not successfully send to any peer.
*/
non_null()
int gc_send_custom_packet(const GC_Chat *chat, bool lossless, const uint8_t *data, uint16_t length);
Expand Down
5 changes: 5 additions & 0 deletions toxcore/tox.c
Original file line number Diff line number Diff line change
Expand Up @@ -4021,6 +4021,11 @@
SET_ERROR_PARAMETER(error, TOX_ERR_GROUP_SEND_CUSTOM_PACKET_PERMISSIONS);
return false;
}

case -4: {
SET_ERROR_PARAMETER(error, TOX_ERR_GROUP_SEND_CUSTOM_PACKET_FAIL_SEND);
return false;

Check warning on line 4027 in toxcore/tox.c

View check run for this annotation

Codecov / codecov/patch

toxcore/tox.c#L4026-L4027

Added lines #L4026 - L4027 were not covered by tests
}
}

/* can't happen */
Expand Down
6 changes: 6 additions & 0 deletions toxcore/tox.h
Original file line number Diff line number Diff line change
Expand Up @@ -4736,6 +4736,12 @@ typedef enum Tox_Err_Group_Send_Custom_Packet {
*/
TOX_ERR_GROUP_SEND_CUSTOM_PACKET_DISCONNECTED,

/**
* The packet did not successfully send to any peer. This often indicates
* a connection issue on the sender's side.
*/
TOX_ERR_GROUP_SEND_CUSTOM_PACKET_FAIL_SEND,

} Tox_Err_Group_Send_Custom_Packet;

const char *tox_err_group_send_custom_packet_to_string(Tox_Err_Group_Send_Custom_Packet value);
Expand Down
3 changes: 3 additions & 0 deletions toxcore/tox_api.c
Original file line number Diff line number Diff line change
Expand Up @@ -1259,6 +1259,9 @@

case TOX_ERR_GROUP_SEND_CUSTOM_PACKET_DISCONNECTED:
return "TOX_ERR_GROUP_SEND_CUSTOM_PACKET_DISCONNECTED";

case TOX_ERR_GROUP_SEND_CUSTOM_PACKET_FAIL_SEND:
return "TOX_ERR_GROUP_SEND_CUSTOM_PACKET_FAIL_SEND";

Check warning on line 1264 in toxcore/tox_api.c

View check run for this annotation

Codecov / codecov/patch

toxcore/tox_api.c#L1264

Added line #L1264 was not covered by tests
}

return "<invalid Tox_Err_Group_Send_Custom_Packet>";
Expand Down
Loading