cleanup: Enable most cppcheck warnings as errors.

Cleaned up some of the warnings it gives. Disabled the ones that are
less useful for us at this time.

auto_tests/auto_test_support.c

@@ -235,7 +235,7 @@ static void autotox_add_friend(AutoTox *autotoxes, uint32_t adding, uint32_t add
 static void initialise_friend_graph(Graph_Type graph, uint32_t num_toxes, AutoTox *autotoxes)
 {
     if (graph == GRAPH_LINEAR) {
-        printf("toxes #%u-#%u each add adjacent toxes as friends\n", 0, num_toxes - 1);
+        printf("toxes #%d-#%u each add adjacent toxes as friends\n", 0, num_toxes - 1);
 
         for (uint32_t i = 0; i < num_toxes; ++i) {
             for (uint32_t j = i - 1; j != i + 3; j += 2) {
@@ -245,7 +245,7 @@ static void initialise_friend_graph(Graph_Type graph, uint32_t num_toxes, AutoTo
             }
         }
     } else if (graph == GRAPH_COMPLETE) {
-        printf("toxes #%u-#%u add each other as friends\n", 0, num_toxes - 1);
+        printf("toxes #%d-#%u add each other as friends\n", 0, num_toxes - 1);
 
         for (uint32_t i = 0; i < num_toxes; ++i) {
             for (uint32_t j = 0; j < num_toxes; ++j) {

auto_tests/auto_test_support.h

@@ -21,17 +21,17 @@ typedef struct AutoTox {
     void *state;
 } AutoTox;
 
-bool all_connected(AutoTox *toxes, uint32_t tox_count);
+bool all_connected(AutoTox *autotoxes, uint32_t tox_count);
 
-bool all_friends_connected(AutoTox *toxes, uint32_t tox_count);
+bool all_friends_connected(AutoTox *autotoxes, uint32_t tox_count);
 
-void iterate_all_wait(AutoTox *toxes, uint32_t tox_count, uint32_t wait);
+void iterate_all_wait(AutoTox *autotoxes, uint32_t tox_count, uint32_t wait);
 
 void save_autotox(AutoTox *autotox);
 void kill_autotox(AutoTox *autotox);
 void reload(AutoTox *autotox);
 
-void set_mono_time_callback(AutoTox *tox);
+void set_mono_time_callback(AutoTox *autotox);
 
 typedef enum Graph_Type {
     GRAPH_COMPLETE = 0,

other/analysis/gen-file.sh

@@ -8,7 +8,6 @@ CPPFLAGS+=("-Iother/bootstrap_daemon/src")
 CPPFLAGS+=("-Iother/fun")
 CPPFLAGS+=("-Itesting")
 CPPFLAGS+=("-Itesting/fuzzing")
-CPPFLAGS+=("-Itesting/groupchats")
 CPPFLAGS+=("-Itoxcore")
 CPPFLAGS+=("-Itoxcore/events")
 CPPFLAGS+=("-Itoxav")

other/analysis/run-cppcheck

@@ -6,9 +6,29 @@ SKIP_GTEST=1
 
 set -e
 
+CPPCHECK="--enable=all"
+CPPCHECK+=("--inconclusive")
+CPPCHECK+=("--error-exitcode=1")
+# Used for VLA.
+CPPCHECK+=("--suppress=allocaCalled")
+# We actually write C code.
+CPPCHECK+=("--suppress=cstyleCast")
+# Used to enable/disable parts using if with constant condition.
+CPPCHECK+=("--suppress=knownConditionTrueFalse")
+# Cppcheck does not need standard library headers to get proper results.
+CPPCHECK+=("--suppress=missingIncludeSystem")
+# Range-for is fine.
+CPPCHECK+=("--suppress=useStlAlgorithm")
+# TODO(iphydf): Maybe fix?
+CPPCHECK+=("--suppress=shadowArgument")
+CPPCHECK+=("--suppress=shadowFunction")
+CPPCHECK+=("--suppress=signConversion")
+# TODO(iphydf): Fixed in the toxav refactor PR.
+CPPCHECK+=("--suppress=redundantAssignment")
+
 run() {
   echo "Running cppcheck in variant '$*'"
-  cppcheck amalgamation.cc "${CPPFLAGS[@]}" "$@"
+  cppcheck "${CPPCHECK[@]}" amalgamation.cc "${CPPFLAGS[@]}" "$@"
 }
 
 . other/analysis/variants.sh

other/fun/create_common.h

@@ -60,7 +60,7 @@ static void handle_args(const int argc, const char *const argv[], const char *co
 
         crypto_scalarmult_base(public_key, secret_key);
     } else if (argc == 2) {
-        printf("Error: Secret key must be a %d character hex string.\n", crypto_box_SECRETKEYBYTES * 2);
+        printf("Error: Secret key must be a %u character hex string.\n", crypto_box_SECRETKEYBYTES * 2);
         exit(1);
     } else {
         print_usage(argv[0], does_what);

toxav/toxav.c

@@ -638,7 +638,7 @@ bool toxav_call_control(ToxAV *av, uint32_t friend_number, Toxav_Call_Control co
 
     return rc == TOXAV_ERR_CALL_CONTROL_OK;
 }
-bool toxav_audio_set_bit_rate(ToxAV *av, uint32_t friend_number, uint32_t audio_bit_rate,
+bool toxav_audio_set_bit_rate(ToxAV *av, uint32_t friend_number, uint32_t bit_rate,
                               Toxav_Err_Bit_Rate_Set *error)
 {
     Toxav_Err_Bit_Rate_Set rc = TOXAV_ERR_BIT_RATE_SET_OK;
@@ -649,7 +649,7 @@ bool toxav_audio_set_bit_rate(ToxAV *av, uint32_t friend_number, uint32_t audio_
         goto RETURN;
     }
 
-    if (audio_bit_rate > 0 && audio_bit_rate_invalid(audio_bit_rate)) {
+    if (bit_rate > 0 && audio_bit_rate_invalid(bit_rate)) {
         rc = TOXAV_ERR_BIT_RATE_SET_INVALID_BIT_RATE;
         goto RETURN;
     }
@@ -663,10 +663,10 @@ bool toxav_audio_set_bit_rate(ToxAV *av, uint32_t friend_number, uint32_t audio_
         goto RETURN;
     }
 
-    LOGGER_DEBUG(av->m->log, "Setting new audio bitrate to: %d", audio_bit_rate);
+    LOGGER_DEBUG(av->m->log, "Setting new audio bitrate to: %d", bit_rate);
 
-    if (call->audio_bit_rate == audio_bit_rate) {
-        LOGGER_DEBUG(av->m->log, "Audio bitrate already set to: %d", audio_bit_rate);
+    if (call->audio_bit_rate == bit_rate) {
+        LOGGER_DEBUG(av->m->log, "Audio bitrate already set to: %d", bit_rate);
     } else if (audio_bit_rate == 0) {
         LOGGER_DEBUG(av->m->log, "Turned off audio sending");
 
@@ -694,10 +694,10 @@ bool toxav_audio_set_bit_rate(ToxAV *av, uint32_t friend_number, uint32_t audio_
                 goto RETURN;
             }
         } else {
-            LOGGER_DEBUG(av->m->log, "Set new audio bit rate %d", audio_bit_rate);
+            LOGGER_DEBUG(av->m->log, "Set new audio bit rate %d", bit_rate);
         }
 
-        call->audio_bit_rate = audio_bit_rate;
+        call->audio_bit_rate = bit_rate;
         pthread_mutex_unlock(call->toxav_call_mutex);
     }
 
@@ -710,7 +710,7 @@ bool toxav_audio_set_bit_rate(ToxAV *av, uint32_t friend_number, uint32_t audio_
 
     return rc == TOXAV_ERR_BIT_RATE_SET_OK;
 }
-bool toxav_video_set_bit_rate(ToxAV *av, uint32_t friend_number, uint32_t video_bit_rate,
+bool toxav_video_set_bit_rate(ToxAV *av, uint32_t friend_number, uint32_t bit_rate,
                               Toxav_Err_Bit_Rate_Set *error)
 {
     Toxav_Err_Bit_Rate_Set rc = TOXAV_ERR_BIT_RATE_SET_OK;
@@ -721,7 +721,7 @@ bool toxav_video_set_bit_rate(ToxAV *av, uint32_t friend_number, uint32_t video_
         goto RETURN;
     }
 
-    if (video_bit_rate > 0 && video_bit_rate_invalid(video_bit_rate)) {
+    if (bit_rate > 0 && video_bit_rate_invalid(bit_rate)) {
         rc = TOXAV_ERR_BIT_RATE_SET_INVALID_BIT_RATE;
         goto RETURN;
     }
@@ -735,10 +735,10 @@ bool toxav_video_set_bit_rate(ToxAV *av, uint32_t friend_number, uint32_t video_
         goto RETURN;
     }
 
-    LOGGER_DEBUG(av->m->log, "Setting new video bitrate to: %d", video_bit_rate);
+    LOGGER_DEBUG(av->m->log, "Setting new video bitrate to: %d", bit_rate);
 
-    if (call->video_bit_rate == video_bit_rate) {
-        LOGGER_DEBUG(av->m->log, "Video bitrate already set to: %d", video_bit_rate);
+    if (call->video_bit_rate == bit_rate) {
+        LOGGER_DEBUG(av->m->log, "Video bitrate already set to: %d", bit_rate);
     } else if (video_bit_rate == 0) {
         LOGGER_DEBUG(av->m->log, "Turned off video sending");
 
@@ -766,10 +766,10 @@ bool toxav_video_set_bit_rate(ToxAV *av, uint32_t friend_number, uint32_t video_
                 goto RETURN;
             }
         } else {
-            LOGGER_DEBUG(av->m->log, "Set new video bit rate %d", video_bit_rate);
+            LOGGER_DEBUG(av->m->log, "Set new video bit rate %d", bit_rate);
         }
 
-        call->video_bit_rate = video_bit_rate;
+        call->video_bit_rate = bit_rate;
         pthread_mutex_unlock(call->toxav_call_mutex);
     }
 

toxcore/LAN_discovery.h

@@ -36,7 +36,7 @@ Broadcast_Info *lan_discovery_init(DHT *dht);
  * Clear packet handlers.
  */
 non_null()
-void lan_discovery_kill(DHT *dht, Broadcast_Info *info);
+void lan_discovery_kill(DHT *dht, Broadcast_Info *broadcast);
 
 /**
  * Is IP a local ip or not.

toxcore/Messenger.c

@@ -1617,15 +1617,12 @@ static void do_reqchunk_filecb(Messenger *m, int32_t friendnumber, void *userdat
  */
 static void break_files(const Messenger *m, int32_t friendnumber)
 {
+    Friend *const friend = &m->friendlist[friendnumber];
+
     // TODO(irungentoo): Inform the client which file transfers get killed with a callback?
     for (uint32_t i = 0; i < MAX_CONCURRENT_FILE_PIPES; ++i) {
-        if (m->friendlist[friendnumber].file_sending[i].status != FILESTATUS_NONE) {
-            m->friendlist[friendnumber].file_sending[i].status = FILESTATUS_NONE;
-        }
-
-        if (m->friendlist[friendnumber].file_receiving[i].status != FILESTATUS_NONE) {
-            m->friendlist[friendnumber].file_receiving[i].status = FILESTATUS_NONE;
-        }
+        friend->file_sending[i].status = FILESTATUS_NONE;
+        friend->file_receiving[i].status = FILESTATUS_NONE;
     }
 }
 

toxcore/bin_unpack.h

@@ -14,7 +14,7 @@ non_null() bool bin_unpack_bool(bool *val, const msgpack_object *obj);
 non_null() bool bin_unpack_u16(uint16_t *val, const msgpack_object *obj);
 non_null() bool bin_unpack_u32(uint32_t *val, const msgpack_object *obj);
 non_null() bool bin_unpack_u64(uint64_t *val, const msgpack_object *obj);
-non_null() bool bin_unpack_bytes(uint8_t **data, size_t *data_length, const msgpack_object *obj);
+non_null() bool bin_unpack_bytes(uint8_t **data_ptr, size_t *data_length_ptr, const msgpack_object *obj);
 non_null() bool bin_unpack_bytes_fixed(uint8_t *data, uint32_t data_length, const msgpack_object *obj);
 
 #endif  // C_TOXCORE_TOXCORE_BIN_UNPACK_H

toxcore/crypto_core.c

@@ -190,10 +190,10 @@ int32_t encrypt_precompute(const uint8_t *public_key, const uint8_t *secret_key,
     return crypto_box_beforenm(shared_key, public_key, secret_key);
 }
 
-int32_t encrypt_data_symmetric(const uint8_t *secret_key, const uint8_t *nonce,
+int32_t encrypt_data_symmetric(const uint8_t *shared_key, const uint8_t *nonce,
                                const uint8_t *plain, size_t length, uint8_t *encrypted)
 {
-    if (length == 0 || !secret_key || !nonce || !plain || !encrypted) {
+    if (length == 0 || !shared_key || !nonce || !plain || !encrypted) {
         return -1;
     }
 
@@ -226,7 +226,7 @@ int32_t encrypt_data_symmetric(const uint8_t *secret_key, const uint8_t *nonce,
     memcpy(temp_plain + crypto_box_ZEROBYTES, plain, length);
 
     if (crypto_box_afternm(temp_encrypted, temp_plain, length + crypto_box_ZEROBYTES, nonce,
-                           secret_key) != 0) {
+                           shared_key) != 0) {
         crypto_free(temp_plain, size_temp_plain);
         crypto_free(temp_encrypted, size_temp_encrypted);
         return -1;
@@ -241,10 +241,10 @@ int32_t encrypt_data_symmetric(const uint8_t *secret_key, const uint8_t *nonce,
     return length + crypto_box_MACBYTES;
 }
 
-int32_t decrypt_data_symmetric(const uint8_t *secret_key, const uint8_t *nonce,
+int32_t decrypt_data_symmetric(const uint8_t *shared_key, const uint8_t *nonce,
                                const uint8_t *encrypted, size_t length, uint8_t *plain)
 {
-    if (length <= crypto_box_BOXZEROBYTES || !secret_key || !nonce || !encrypted || !plain) {
+    if (length <= crypto_box_BOXZEROBYTES || !shared_key || !nonce || !encrypted || !plain) {
         return -1;
     }
 
@@ -275,7 +275,7 @@ int32_t decrypt_data_symmetric(const uint8_t *secret_key, const uint8_t *nonce,
     memcpy(temp_encrypted + crypto_box_BOXZEROBYTES, encrypted, length);
 
     if (crypto_box_open_afternm(temp_plain, temp_encrypted, length + crypto_box_BOXZEROBYTES, nonce,
-                                secret_key) != 0) {
+                                shared_key) != 0) {
         crypto_free(temp_plain, size_temp_plain);
         crypto_free(temp_encrypted, size_temp_encrypted);
         return -1;
@@ -395,11 +395,11 @@ void crypto_sha512(uint8_t *hash, const uint8_t *data, size_t length)
     crypto_hash_sha512(hash, data, length);
 }
 
-void random_bytes(uint8_t *data, size_t length)
+void random_bytes(uint8_t *bytes, size_t length)
 {
 #ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
-    fuzz_random_bytes(data, length);
+    fuzz_random_bytes(bytes, length);
 #else
-    randombytes(data, length);
+    randombytes(bytes, length);
 #endif
 }

toxcore/events/conference_invite.c

@@ -147,14 +147,13 @@ static Tox_Event_Conference_Invite *tox_events_add_conference_invite(Tox_Events
 
     if (events->conference_invite_size == events->conference_invite_capacity) {
         const uint32_t new_conference_invite_capacity = events->conference_invite_capacity * 2 + 1;
-        Tox_Event_Conference_Invite *new_conference_invite = (Tox_Event_Conference_Invite *)realloc(
+        events->conference_invite = (Tox_Event_Conference_Invite *)realloc(
                     events->conference_invite, new_conference_invite_capacity * sizeof(Tox_Event_Conference_Invite));
 
-        if (new_conference_invite == nullptr) {
+        if (events->conference_invite == nullptr) {
             return nullptr;
         }
 
-        events->conference_invite = new_conference_invite;
         events->conference_invite_capacity = new_conference_invite_capacity;
     }
 

toxcore/events/friend_status.c

@@ -25,7 +25,7 @@
 
 struct Tox_Event_Friend_Status {
     uint32_t friend_number;
-    Tox_User_Status connection_status;
+    Tox_User_Status status;
 };
 
 non_null()
@@ -55,16 +55,16 @@ uint32_t tox_event_friend_status_get_friend_number(const Tox_Event_Friend_Status
 }
 
 non_null()
-static void tox_event_friend_status_set_connection_status(Tox_Event_Friend_Status *friend_status,
-        Tox_User_Status connection_status)
+static void tox_event_friend_status_set_status(Tox_Event_Friend_Status *friend_status,
+        Tox_User_Status status)
 {
     assert(friend_status != nullptr);
-    friend_status->connection_status = connection_status;
+    friend_status->status = status;
 }
-Tox_User_Status tox_event_friend_status_get_connection_status(const Tox_Event_Friend_Status *friend_status)
+Tox_User_Status tox_event_friend_status_get_status(const Tox_Event_Friend_Status *friend_status)
 {
     assert(friend_status != nullptr);
-    return friend_status->connection_status;
+    return friend_status->status;
 }
 
 non_null()
@@ -76,7 +76,7 @@ static void tox_event_friend_status_pack(
     bin_pack_u32(mp, TOX_EVENT_FRIEND_STATUS);
     bin_pack_array(mp, 2);
     bin_pack_u32(mp, event->friend_number);
-    bin_pack_u32(mp, event->connection_status);
+    bin_pack_u32(mp, event->status);
 }
 
 non_null()
@@ -90,7 +90,7 @@ static bool tox_event_friend_status_unpack(
     }
 
     return bin_unpack_u32(&event->friend_number, &obj->via.array.ptr[0])
-           && tox_unpack_user_status(&event->connection_status, &obj->via.array.ptr[1]);
+           && tox_unpack_user_status(&event->status, &obj->via.array.ptr[1]);
 }
 
 
@@ -187,7 +187,7 @@ bool tox_events_unpack_friend_status(Tox_Events *events, const msgpack_object *o
  *****************************************************/
 
 
-void tox_events_handle_friend_status(Tox *tox, uint32_t friend_number, Tox_User_Status connection_status,
+void tox_events_handle_friend_status(Tox *tox, uint32_t friend_number, Tox_User_Status status,
                                      void *user_data)
 {
     Tox_Events_State *state = tox_events_alloc(user_data);
@@ -201,5 +201,5 @@ void tox_events_handle_friend_status(Tox *tox, uint32_t friend_number, Tox_User_
     }
 
     tox_event_friend_status_set_friend_number(friend_status, friend_number);
-    tox_event_friend_status_set_connection_status(friend_status, connection_status);
+    tox_event_friend_status_set_status(friend_status, status);
 }