cleanup: Fix all -Wsign-compare warnings.

Author: iphydf

.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.

.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

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");

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,

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,

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 \

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);

other/fun/save-generator.c

@@ -137,15 +137,15 @@ 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),
                        &friend_err);
         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);
         }
     }
 

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]);
         }
 

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;

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);
     }

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 || (uint32_t)len != data_size) {
         return -1;
     }
 

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;
     }
 

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;

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;
     }
 

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;

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;
     }