From 1a04a2a21fa2d9b857f7add17acb876357638819 Mon Sep 17 00:00:00 2001 From: iphydf Date: Sun, 2 Nov 2025 23:48:22 +0000 Subject: [PATCH] perf: Make the code for adding group peers non-recursive. Replaces the recursive implementation of add_to_closest in with an equivalent iterative loop. This eliminates the risk of stack overflow during deep peer eviction chains. --- auto_tests/auto_test_support.c | 54 +++++++---- .../scenario_toxav_peer_offline_test.c | 2 +- testing/BUILD.bazel | 1 + testing/support/BUILD.bazel | 4 - toxcore/ev_test_util.cc | 13 +-- toxcore/group.c | 92 ++++++++++--------- 6 files changed, 94 insertions(+), 72 deletions(-) diff --git a/auto_tests/auto_test_support.c b/auto_tests/auto_test_support.c index 4086f617d0..36fe8b4cfc 100644 --- a/auto_tests/auto_test_support.c +++ b/auto_tests/auto_test_support.c @@ -49,32 +49,46 @@ static const struct BootstrapNodes { const uint8_t key[32]; } bootstrap_nodes[] = { { - "tox.abilinski.com", 33445, - 0x10, 0xC0, 0x0E, 0xB2, 0x50, 0xC3, 0x23, 0x3E, - 0x34, 0x3E, 0x2A, 0xEB, 0xA0, 0x71, 0x15, 0xA5, - 0xC2, 0x89, 0x20, 0xE9, 0xC8, 0xD2, 0x94, 0x92, - 0xF6, 0xD0, 0x0B, 0x29, 0x04, 0x9E, 0xDC, 0x7E, + "144.217.167.73", 33445, + 0x7E, 0x56, 0x68, 0xE0, 0xEE, 0x09, 0xE1, 0x9F, + 0x32, 0x0A, 0xD4, 0x79, 0x02, 0x41, 0x93, 0x31, + 0xFF, 0xEE, 0x14, 0x7B, 0xB3, 0x60, 0x67, 0x69, + 0xCF, 0xBE, 0x92, 0x1A, 0x2A, 0x2F, 0xD3, 0x4C, }, { - "tox.initramfs.io", 33445, - 0x02, 0x80, 0x7C, 0xF4, 0xF8, 0xBB, 0x8F, 0xB3, - 0x90, 0xCC, 0x37, 0x94, 0xBD, 0xF1, 0xE8, 0x44, - 0x9E, 0x9A, 0x83, 0x92, 0xC5, 0xD3, 0xF2, 0x20, - 0x00, 0x19, 0xDA, 0x9F, 0x1E, 0x81, 0x2E, 0x46, + "205.185.115.131", 33445, + 0x30, 0x91, 0xC6, 0xBE, 0xB2, 0xA9, 0x93, 0xF1, + 0xC6, 0x30, 0x0C, 0x16, 0x54, 0x9F, 0xAB, 0xA6, + 0x70, 0x98, 0xFF, 0x3D, 0x62, 0xC6, 0xD2, 0x53, + 0x82, 0x8B, 0x53, 0x14, 0x70, 0xB5, 0x3D, 0x68, }, { - "tox.plastiras.org", 33445, - 0x8E, 0x8B, 0x63, 0x29, 0x9B, 0x3D, 0x52, 0x0F, - 0xB3, 0x77, 0xFE, 0x51, 0x00, 0xE6, 0x5E, 0x33, - 0x22, 0xF7, 0xAE, 0x5B, 0x20, 0xA0, 0xAC, 0xED, - 0x29, 0x81, 0x76, 0x9F, 0xC5, 0xB4, 0x37, 0x25, + "tox1.mf-net.eu", 33445, + 0xB3, 0xE5, 0xFA, 0x80, 0xDC, 0x8E, 0xBD, 0x11, + 0x49, 0xAD, 0x2A, 0xB3, 0x5E, 0xD8, 0xB8, 0x5B, + 0xD5, 0x46, 0xDE, 0xDE, 0x26, 0x1C, 0xA5, 0x93, + 0x23, 0x4C, 0x61, 0x92, 0x49, 0x41, 0x95, 0x06, }, { - "tox.novg.net", 33445, - 0xD5, 0x27, 0xE5, 0x84, 0x7F, 0x83, 0x30, 0xD6, - 0x28, 0xDA, 0xB1, 0x81, 0x4F, 0x0A, 0x42, 0x2F, - 0x6D, 0xC9, 0xD0, 0xA3, 0x00, 0xE6, 0xC3, 0x57, - 0x63, 0x4E, 0xE2, 0xDA, 0x88, 0xC3, 0x54, 0x63, + "3.0.24.15", 33445, + 0xE2, 0x0A, 0xBC, 0xF3, 0x8C, 0xDB, 0xFF, 0xD7, + 0xD0, 0x4B, 0x29, 0xC9, 0x56, 0xB3, 0x3F, 0x7B, + 0x27, 0xA3, 0xBB, 0x7A, 0xF0, 0x61, 0x81, 0x01, + 0x61, 0x7B, 0x03, 0x6E, 0x4A, 0xEA, 0x40, 0x2D, + }, + { + "139.162.110.188", 33445, + 0xF7, 0x6A, 0x11, 0x28, 0x45, 0x47, 0x16, 0x38, + 0x89, 0xDD, 0xC8, 0x9A, 0x77, 0x38, 0xCF, 0x27, + 0x17, 0x97, 0xBF, 0x5E, 0x5E, 0x22, 0x06, 0x43, + 0xE9, 0x7A, 0xD3, 0xC7, 0xE7, 0x90, 0x3D, 0x55, + }, + { + "172.105.109.31", 33445, + 0xD4, 0x6E, 0x97, 0xCF, 0x99, 0x5D, 0xC1, 0x82, + 0x0B, 0x92, 0xB7, 0xD8, 0x99, 0xE1, 0x52, 0xA2, + 0x17, 0xD3, 0x6A, 0xBE, 0x22, 0x73, 0x0F, 0xEA, + 0x4B, 0x6B, 0xF1, 0xBF, 0xC0, 0x6C, 0x61, 0x7C, }, { nullptr, 0, 0 }, }; diff --git a/auto_tests/scenarios/scenario_toxav_peer_offline_test.c b/auto_tests/scenarios/scenario_toxav_peer_offline_test.c index 62ac4e75be..54bcbdbedc 100644 --- a/auto_tests/scenarios/scenario_toxav_peer_offline_test.c +++ b/auto_tests/scenarios/scenario_toxav_peer_offline_test.c @@ -133,7 +133,7 @@ int main(int argc, char *argv[]) ToxScenarioStatus res = tox_scenario_run(s); if (res != TOX_SCENARIO_DONE) { - fprintf(stderr, "Scenario failed with status %d\n", res); + fprintf(stderr, "Scenario failed with status %u\n", res); return 1; } diff --git a/testing/BUILD.bazel b/testing/BUILD.bazel index c1daebbcf8..a896ed7e1d 100644 --- a/testing/BUILD.bazel +++ b/testing/BUILD.bazel @@ -15,6 +15,7 @@ sh_test( args = ["$(locations %s)" % f for f in CIMPLE_FILES] + [ "-Wno-boolean-return", "-Wno-callback-names", + "-Wno-callgraph", "-Wno-enum-from-int", "-Wno-nullability", "-Wno-ownership-decls", diff --git a/testing/support/BUILD.bazel b/testing/support/BUILD.bazel index c26265adaf..50b71dbe43 100644 --- a/testing/support/BUILD.bazel +++ b/testing/support/BUILD.bazel @@ -40,10 +40,6 @@ cc_library( "public/tox_network.hh", "public/tox_runner.hh", ], - copts = select({ - "//tools/config:windows": ["/wd4200"], # Zero-sized array in struct/union - "//conditions:default": [], - }), visibility = ["//visibility:public"], deps = [ "//c-toxcore/toxcore:attributes", diff --git a/toxcore/ev_test_util.cc b/toxcore/ev_test_util.cc index de312413f9..806054599f 100644 --- a/toxcore/ev_test_util.cc +++ b/toxcore/ev_test_util.cc @@ -28,7 +28,7 @@ int create_pair(Socket *rs, Socket *ws) addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK); addr.sin_port = 0; - if (bind(listener, (struct sockaddr *)&addr, sizeof(addr)) != 0) { + if (bind(listener, reinterpret_cast(&addr), sizeof(addr)) != 0) { closesocket(listener); return -1; } @@ -39,7 +39,7 @@ int create_pair(Socket *rs, Socket *ws) } socklen_t len = sizeof(addr); - if (getsockname(listener, (struct sockaddr *)&addr, &len) != 0) { + if (getsockname(listener, reinterpret_cast(&addr), &len) != 0) { closesocket(listener); return -1; } @@ -50,7 +50,7 @@ int create_pair(Socket *rs, Socket *ws) return -1; } - if (connect(client, (struct sockaddr *)&addr, sizeof(addr)) != 0) { + if (connect(client, reinterpret_cast(&addr), sizeof(addr)) != 0) { closesocket(client); closesocket(listener); return -1; @@ -65,8 +65,8 @@ int create_pair(Socket *rs, Socket *ws) closesocket(listener); - *rs = net_socket_from_native((int)client); - *ws = net_socket_from_native((int)server); + *rs = net_socket_from_native(static_cast(client)); + *ws = net_socket_from_native(static_cast(server)); return 0; } @@ -80,7 +80,8 @@ void close_pair(Socket s1, Socket s2) int write_socket(Socket s, const void *buf, std::size_t count) { - return send(net_socket_to_native(s), (const char *)buf, (int)count, 0); + const char *data = reinterpret_cast(buf); + return send(net_socket_to_native(s), data, static_cast(count), 0); } #else int create_pair(Socket *rs, Socket *ws) diff --git a/toxcore/group.c b/toxcore/group.c index 8c782ddb5e..f6e4c3a759 100644 --- a/toxcore/group.c +++ b/toxcore/group.c @@ -442,73 +442,83 @@ static bool add_to_closest(Group_c *_Nonnull g, const uint8_t *_Nonnull real_pk, return false; } - unsigned int index = DESIRED_CLOSEST; - for (unsigned int i = 0; i < DESIRED_CLOSEST; ++i) { if (g->closest_peers[i].active && pk_equal(real_pk, g->closest_peers[i].real_pk)) { return true; } } - for (unsigned int i = 0; i < DESIRED_CLOSEST; ++i) { - if (!g->closest_peers[i].active) { - index = i; - break; - } - } + uint8_t cur_real_pk[CRYPTO_PUBLIC_KEY_SIZE]; + uint8_t cur_temp_pk[CRYPTO_PUBLIC_KEY_SIZE]; + memcpy(cur_real_pk, real_pk, CRYPTO_PUBLIC_KEY_SIZE); + memcpy(cur_temp_pk, temp_pk, CRYPTO_PUBLIC_KEY_SIZE); - if (index == DESIRED_CLOSEST) { - uint64_t comp_val = calculate_comp_value(g->real_pk, real_pk); - uint64_t comp_d = 0; + bool added = false; - for (unsigned int i = 0; i < (DESIRED_CLOSEST / 2); ++i) { - const uint64_t comp = calculate_comp_value(g->real_pk, g->closest_peers[i].real_pk); + while (true) { + unsigned int index = DESIRED_CLOSEST; - if (comp > comp_val && comp > comp_d) { + for (unsigned int i = 0; i < DESIRED_CLOSEST; ++i) { + if (!g->closest_peers[i].active) { index = i; - comp_d = comp; + break; } } - comp_val = calculate_comp_value(real_pk, g->real_pk); + if (index == DESIRED_CLOSEST) { + uint64_t comp_val = calculate_comp_value(g->real_pk, cur_real_pk); + uint64_t comp_d = 0; - for (unsigned int i = DESIRED_CLOSEST / 2; i < DESIRED_CLOSEST; ++i) { - const uint64_t comp = calculate_comp_value(g->closest_peers[i].real_pk, g->real_pk); + for (unsigned int i = 0; i < (DESIRED_CLOSEST / 2); ++i) { + const uint64_t comp = calculate_comp_value(g->real_pk, g->closest_peers[i].real_pk); - if (comp > comp_val && comp > comp_d) { - index = i; - comp_d = comp; + if (comp > comp_val && comp > comp_d) { + index = i; + comp_d = comp; + } } - } - } - if (index == DESIRED_CLOSEST) { - return false; - } + comp_val = calculate_comp_value(cur_real_pk, g->real_pk); - uint8_t old_real_pk[CRYPTO_PUBLIC_KEY_SIZE]; - uint8_t old_temp_pk[CRYPTO_PUBLIC_KEY_SIZE]; - bool old = false; + for (unsigned int i = DESIRED_CLOSEST / 2; i < DESIRED_CLOSEST; ++i) { + const uint64_t comp = calculate_comp_value(g->closest_peers[i].real_pk, g->real_pk); - if (g->closest_peers[index].active) { - memcpy(old_real_pk, g->closest_peers[index].real_pk, CRYPTO_PUBLIC_KEY_SIZE); - memcpy(old_temp_pk, g->closest_peers[index].temp_pk, CRYPTO_PUBLIC_KEY_SIZE); - old = true; - } + if (comp > comp_val && comp > comp_d) { + index = i; + comp_d = comp; + } + } + } + + if (index == DESIRED_CLOSEST) { + break; + } - g->closest_peers[index].active = true; - memcpy(g->closest_peers[index].real_pk, real_pk, CRYPTO_PUBLIC_KEY_SIZE); - memcpy(g->closest_peers[index].temp_pk, temp_pk, CRYPTO_PUBLIC_KEY_SIZE); + if (g->closest_peers[index].active) { + uint8_t tmp[CRYPTO_PUBLIC_KEY_SIZE]; + memcpy(tmp, g->closest_peers[index].real_pk, CRYPTO_PUBLIC_KEY_SIZE); + memcpy(g->closest_peers[index].real_pk, cur_real_pk, CRYPTO_PUBLIC_KEY_SIZE); + memcpy(cur_real_pk, tmp, CRYPTO_PUBLIC_KEY_SIZE); - if (old) { - add_to_closest(g, old_real_pk, old_temp_pk); + memcpy(tmp, g->closest_peers[index].temp_pk, CRYPTO_PUBLIC_KEY_SIZE); + memcpy(g->closest_peers[index].temp_pk, cur_temp_pk, CRYPTO_PUBLIC_KEY_SIZE); + memcpy(cur_temp_pk, tmp, CRYPTO_PUBLIC_KEY_SIZE); + } else { + g->closest_peers[index].active = true; + memcpy(g->closest_peers[index].real_pk, cur_real_pk, CRYPTO_PUBLIC_KEY_SIZE); + memcpy(g->closest_peers[index].temp_pk, cur_temp_pk, CRYPTO_PUBLIC_KEY_SIZE); + added = true; + break; + } + + added = true; } - if (g->changed == GROUPCHAT_CLOSEST_CHANGE_NONE) { + if (added && g->changed == GROUPCHAT_CLOSEST_CHANGE_NONE) { g->changed = GROUPCHAT_CLOSEST_CHANGE_ADDED; } - return true; + return added; } static bool pk_in_closest_peers(const Group_c *_Nonnull g, const uint8_t *_Nonnull real_pk)