From 211a7aba7dbb3ea22ce76b37157ccdd016d9c072 Mon Sep 17 00:00:00 2001 From: iphydf Date: Fri, 8 Nov 2024 17:14:48 +0000 Subject: [PATCH] cleanup: Fix all `-Wsign-compare` warnings. --- .github/scripts/flags-clang.sh | 3 --- .github/scripts/flags-gcc.sh | 3 +-- auto_tests/conference_av_test.c | 2 +- auto_tests/conference_double_invite_test.c | 2 +- auto_tests/conference_invite_merge_test.c | 2 +- other/analysis/run-clang | 1 - other/bootstrap_daemon/src/tox-bootstrapd.c | 2 +- other/fun/save-generator.c | 4 ++-- other/fun/sign.c | 6 +++--- other/fun/strkey.c | 2 +- toxav/groupav.c | 4 ++-- toxcore/DHT.c | 10 +++++----- toxcore/announce.c | 2 +- toxcore/group.c | 4 ++-- toxcore/group_chats.c | 2 +- toxcore/onion_client.c | 2 +- toxencryptsave/toxencryptsave.c | 8 ++++---- 17 files changed, 27 insertions(+), 32 deletions(-) diff --git a/.github/scripts/flags-clang.sh b/.github/scripts/flags-clang.sh index d0a7729813..f0d9a11c87 100644 --- a/.github/scripts/flags-clang.sh +++ b/.github/scripts/flags-clang.sh @@ -36,9 +36,6 @@ add_flag -Wno-padded # This warns on things like _XOPEN_SOURCE, which we currently need (we # probably won't need these in the future). add_flag -Wno-reserved-id-macro -# TODO(iphydf): Clean these up. They are likely not bugs, but still -# potential issues and probably confusing. -add_flag -Wno-sign-compare # We don't want to have default cases, we want to explicitly define all cases add_flag -Wno-switch-default # __attribute__((nonnull)) causes this warning on defensive null checks. diff --git a/.github/scripts/flags-gcc.sh b/.github/scripts/flags-gcc.sh index 0cacdb4b17..50a0531ac9 100644 --- a/.github/scripts/flags-gcc.sh +++ b/.github/scripts/flags-gcc.sh @@ -47,8 +47,7 @@ add_flag -Wunused-value # struct Foo foo = {0}; is a common idiom. add_flag -Wno-missing-field-initializers -# TODO(iphydf): Clean these up. They are likely not bugs, but still -# potential issues and probably confusing. +# Checked by clang, but gcc is warning when it's not necessary. add_flag -Wno-sign-compare # File transfer code has this. add_flag -Wno-type-limits diff --git a/auto_tests/conference_av_test.c b/auto_tests/conference_av_test.c index 39383bd74c..28a2a3a1e3 100644 --- a/auto_tests/conference_av_test.c +++ b/auto_tests/conference_av_test.c @@ -420,7 +420,7 @@ static void test_groupav(AutoTox *autotoxes) tox_events_callback_conference_connected(autotoxes[i].dispatch, handle_conference_connected); } - ck_assert_msg(toxav_add_av_groupchat(autotoxes[0].tox, audio_callback, &autotoxes[0]) != UINT32_MAX, + ck_assert_msg(toxav_add_av_groupchat(autotoxes[0].tox, audio_callback, &autotoxes[0]) != -1, "failed to create group"); printf("tox #%u: inviting its first friend\n", autotoxes[0].index); ck_assert_msg(tox_conference_invite(autotoxes[0].tox, 0, 0, nullptr) != 0, "failed to invite friend"); diff --git a/auto_tests/conference_double_invite_test.c b/auto_tests/conference_double_invite_test.c index 06d880a20d..4d16c8158b 100644 --- a/auto_tests/conference_double_invite_test.c +++ b/auto_tests/conference_double_invite_test.c @@ -28,7 +28,7 @@ static void handle_conference_invite( ck_assert_msg(!state->joined, "invitation callback generated for already joined conference"); - if (friend_number != -1) { + if (friend_number != UINT32_MAX) { Tox_Err_Conference_Join err; state->conference = tox_conference_join(autotox->tox, friend_number, cookie, length, &err); ck_assert_msg(err == TOX_ERR_CONFERENCE_JOIN_OK, diff --git a/auto_tests/conference_invite_merge_test.c b/auto_tests/conference_invite_merge_test.c index d0af332d22..536f47fd74 100644 --- a/auto_tests/conference_invite_merge_test.c +++ b/auto_tests/conference_invite_merge_test.c @@ -21,7 +21,7 @@ static void handle_conference_invite( const uint8_t *cookie = tox_event_conference_invite_get_cookie(event); const size_t length = tox_event_conference_invite_get_cookie_length(event); - if (friend_number != -1) { + if (friend_number != UINT32_MAX) { Tox_Err_Conference_Join err; state->conference = tox_conference_join(autotox->tox, friend_number, cookie, length, &err); ck_assert_msg(err == TOX_ERR_CONFERENCE_JOIN_OK, diff --git a/other/analysis/run-clang b/other/analysis/run-clang index a8c85a983a..3f7113b49a 100755 --- a/other/analysis/run-clang +++ b/other/analysis/run-clang @@ -27,7 +27,6 @@ run() { -Wno-missing-noreturn \ -Wno-old-style-cast \ -Wno-padded \ - -Wno-sign-compare \ -Wno-switch-default \ -Wno-tautological-pointer-compare \ -Wno-unreachable-code-return \ diff --git a/other/bootstrap_daemon/src/tox-bootstrapd.c b/other/bootstrap_daemon/src/tox-bootstrapd.c index 7ca242a7cd..60e14c08f5 100644 --- a/other/bootstrap_daemon/src/tox-bootstrapd.c +++ b/other/bootstrap_daemon/src/tox-bootstrapd.c @@ -618,7 +618,7 @@ int main(int argc, char *argv[]) break; default: - log_write(LOG_LEVEL_INFO, "Received (%d) signal. Exiting.\n", caught_signal); + log_write(LOG_LEVEL_INFO, "Received (%ld) signal. Exiting.\n", (long)caught_signal); } lan_discovery_kill(broadcast); diff --git a/other/fun/save-generator.c b/other/fun/save-generator.c index 9eb556899a..d27d69e6a1 100644 --- a/other/fun/save-generator.c +++ b/other/fun/save-generator.c @@ -137,7 +137,7 @@ int main(int argc, char *argv[]) printf("Failed to set status. Error number: %d\n", err); } - for (unsigned int i = 2; i < argc; i++) { //start at 2 because that is where the tox ids are + for (int i = 2; i < argc; i++) { //start at 2 because that is where the tox ids are uint8_t *address = hex_string_to_bin(argv[i]); Tox_Err_Friend_Add friend_err; tox_friend_add(tox, address, (const uint8_t *)GENERATED_REQUEST_MESSAGE, strlen(GENERATED_REQUEST_MESSAGE), @@ -145,7 +145,7 @@ int main(int argc, char *argv[]) free(address); if (friend_err != TOX_ERR_FRIEND_ADD_OK) { - printf("Failed to add friend number %u. Error number: %d\n", i - 1, friend_err); + printf("Failed to add friend number %d. Error number: %d\n", i - 1, friend_err); } } diff --git a/other/fun/sign.c b/other/fun/sign.c index d38dd77d4e..bf949de08b 100644 --- a/other/fun/sign.c +++ b/other/fun/sign.c @@ -35,7 +35,7 @@ static int load_file(const char *filename, unsigned char **result) fseek(f, 0, SEEK_SET); *result = (unsigned char *)malloc(size + 1); - if (size != fread(*result, sizeof(char), size, f)) { + if ((size_t)size != fread(*result, sizeof(char), size, f)) { free(*result); fclose(f); return -2; // -2 means file reading fail @@ -55,13 +55,13 @@ int main(int argc, char *argv[]) crypto_sign_ed25519_keypair(pk, sk); printf("Public key:\n"); - for (int i = 0; i < crypto_sign_ed25519_PUBLICKEYBYTES; ++i) { + for (uint32_t i = 0; i < crypto_sign_ed25519_PUBLICKEYBYTES; ++i) { printf("%02X", pk[i]); } printf("\nSecret key:\n"); - for (int i = 0; i < crypto_sign_ed25519_SECRETKEYBYTES; ++i) { + for (uint32_t i = 0; i < crypto_sign_ed25519_SECRETKEYBYTES; ++i) { printf("%02X", sk[i]); } diff --git a/other/fun/strkey.c b/other/fun/strkey.c index 538f59ade4..36aa3242c9 100644 --- a/other/fun/strkey.c +++ b/other/fun/strkey.c @@ -117,7 +117,7 @@ int main(int argc, char *argv[]) #endif crypto_box_keypair(public_key, secret_key); - for (int i = 0; i <= crypto_box_PUBLICKEYBYTES - len; ++i) { + for (uint32_t i = 0; i <= crypto_box_PUBLICKEYBYTES - len; ++i) { if (memcmp(public_key + i, desired_bin, len) == 0) { found = 1; break; diff --git a/toxav/groupav.c b/toxav/groupav.c index 63dd569df6..14ca9bb293 100644 --- a/toxav/groupav.c +++ b/toxav/groupav.c @@ -476,7 +476,7 @@ int groupchat_enable_av(const Logger *log, Tox *tox, Group_Chats *g_c, uint32_t return -1; } - for (uint32_t i = 0; i < numpeers; ++i) { + for (uint32_t i = 0; i < (uint32_t)numpeers; ++i) { group_av_peer_new(group_av, conference_number, i); } @@ -508,7 +508,7 @@ int groupchat_disable_av(const Group_Chats *g_c, uint32_t conference_number) return -1; } - for (uint32_t i = 0; i < numpeers; ++i) { + for (uint32_t i = 0; i < (uint32_t)numpeers; ++i) { group_av_peer_delete(group_av, conference_number, group_peer_get_object(g_c, conference_number, i)); group_peer_set_object(g_c, conference_number, i, nullptr); } diff --git a/toxcore/DHT.c b/toxcore/DHT.c index 2567d1b5a8..a081462fc5 100644 --- a/toxcore/DHT.c +++ b/toxcore/DHT.c @@ -380,12 +380,12 @@ int dht_create_packet(const Memory *mem, const Random *rng, const int encrypted_length = encrypt_data_symmetric(shared_key, nonce, plain, plain_length, encrypted); - if (encrypted_length == -1) { + if (encrypted_length < 0) { mem_delete(mem, encrypted); return -1; } - if (length < 1 + CRYPTO_PUBLIC_KEY_SIZE + CRYPTO_NONCE_SIZE + encrypted_length) { + if (length < 1 + CRYPTO_PUBLIC_KEY_SIZE + CRYPTO_NONCE_SIZE + (size_t)encrypted_length) { mem_delete(mem, encrypted); return -1; } @@ -1347,15 +1347,15 @@ static int sendnodes_ipv6(const DHT *dht, const IP_Port *ip_port, const uint8_t plain[0] = num_nodes; memcpy(plain + 1 + nodes_length, sendback_data, length); - const uint32_t crypto_size = 1 + CRYPTO_PUBLIC_KEY_SIZE + CRYPTO_NONCE_SIZE + CRYPTO_MAC_SIZE; - const uint32_t data_size = 1 + nodes_length + length + crypto_size; + const uint16_t crypto_size = 1 + CRYPTO_PUBLIC_KEY_SIZE + CRYPTO_NONCE_SIZE + CRYPTO_MAC_SIZE; + const uint16_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, data_size); - if (len != data_size) { + if (len < 0 || (uint16_t)len != data_size) { return -1; } diff --git a/toxcore/announce.c b/toxcore/announce.c index b983cb0574..7bda993232 100644 --- a/toxcore/announce.c +++ b/toxcore/announce.c @@ -579,7 +579,7 @@ static int create_reply(Announcements *announce, const IP_Port *source, const int plain_reply_max_len = (int)reply_max_length - (1 + CRYPTO_PUBLIC_KEY_SIZE + CRYPTO_NONCE_SIZE + CRYPTO_MAC_SIZE); - if (plain_reply_max_len < sizeof(uint64_t)) { + if (plain_reply_max_len < (int)sizeof(uint64_t)) { return -1; } diff --git a/toxcore/group.c b/toxcore/group.c index 14e61e6ffc..3d96b962de 100644 --- a/toxcore/group.c +++ b/toxcore/group.c @@ -2520,7 +2520,7 @@ static int handle_send_peers(Group_Chats *g_c, uint32_t groupnumber, const uint8 non_null(1, 3) nullable(6) static void handle_direct_packet(Group_Chats *g_c, uint32_t groupnumber, const uint8_t *data, uint16_t length, - int connection_index, void *userdata) + uint32_t connection_index, void *userdata) { if (length == 0) { return; @@ -2832,7 +2832,7 @@ static bool check_message_info(uint32_t message_number, uint8_t message_id, Grou non_null(1, 3) nullable(6) static void handle_message_packet_group(Group_Chats *g_c, uint32_t groupnumber, const uint8_t *data, uint16_t length, - int connection_index, void *userdata) + uint32_t connection_index, void *userdata) { if (length < sizeof(uint16_t) + sizeof(uint32_t) + 1) { return; diff --git a/toxcore/group_chats.c b/toxcore/group_chats.c index 03077d0806..2d64ae60ec 100644 --- a/toxcore/group_chats.c +++ b/toxcore/group_chats.c @@ -8353,7 +8353,7 @@ bool gc_group_is_valid(const GC_Chat *chat) /** Return true if `group_number` designates an active group in session `c`. */ static bool group_number_valid(const GC_Session *c, int group_number) { - if (group_number < 0 || group_number >= c->chats_index) { + if (group_number < 0 || (uint32_t)group_number >= c->chats_index) { return false; } diff --git a/toxcore/onion_client.c b/toxcore/onion_client.c index 9b0ac96102..326575963b 100644 --- a/toxcore/onion_client.c +++ b/toxcore/onion_client.c @@ -955,7 +955,7 @@ static int handle_announce_response(void *object, const IP_Port *source, const u } uint8_t plain[1 + ONION_PING_ID_SIZE + ONION_ANNOUNCE_RESPONSE_MAX_SIZE - ONION_ANNOUNCE_RESPONSE_MIN_SIZE]; - const int plain_size = 1 + ONION_PING_ID_SIZE + length - ONION_ANNOUNCE_RESPONSE_MIN_SIZE; + const uint32_t plain_size = 1 + ONION_PING_ID_SIZE + length - ONION_ANNOUNCE_RESPONSE_MIN_SIZE; int len; const uint16_t nonce_start = 1 + ONION_ANNOUNCE_SENDBACK_DATA_LENGTH; const uint16_t ciphertext_start = nonce_start + CRYPTO_NONCE_SIZE; diff --git a/toxencryptsave/toxencryptsave.c b/toxencryptsave/toxencryptsave.c index f3b88770c1..b785c26732 100644 --- a/toxencryptsave/toxencryptsave.c +++ b/toxencryptsave/toxencryptsave.c @@ -231,8 +231,8 @@ bool tox_pass_key_encrypt(const Tox_Pass_Key *key, const uint8_t plaintext[], si ciphertext += crypto_box_NONCEBYTES; /* now encrypt */ - if (encrypt_data_symmetric(key->key, nonce, plaintext, plaintext_len, ciphertext) - != plaintext_len + crypto_box_MACBYTES) { + const int32_t encrypted_len = encrypt_data_symmetric(key->key, nonce, plaintext, plaintext_len, ciphertext); + if (encrypted_len < 0 || (size_t)encrypted_len != plaintext_len + crypto_box_MACBYTES) { SET_ERROR_PARAMETER(error, TOX_ERR_ENCRYPTION_FAILED); return false; } @@ -316,8 +316,8 @@ bool tox_pass_key_decrypt(const Tox_Pass_Key *key, const uint8_t ciphertext[], s ciphertext += crypto_box_NONCEBYTES; /* decrypt the ciphertext */ - if (decrypt_data_symmetric(key->key, nonce, ciphertext, decrypt_length + crypto_box_MACBYTES, plaintext) - != decrypt_length) { + const int32_t decrypted_len = decrypt_data_symmetric(key->key, nonce, ciphertext, decrypt_length + crypto_box_MACBYTES, plaintext); + if (decrypted_len < 0 || (size_t)decrypted_len != decrypt_length) { SET_ERROR_PARAMETER(error, TOX_ERR_DECRYPTION_FAILED); return false; }