cleanup: Split the huge TCP client packet handler.

Also enable cppcheck in both C and C++ mode.

other/analysis/run-cppcheck

@@ -6,29 +6,34 @@ SKIP_GTEST=1
 
 set -e
 
-CPPCHECK="--enable=all"
+CPPCHECK=("--enable=all")
 CPPCHECK+=("--inconclusive")
 CPPCHECK+=("--error-exitcode=1")
 # Used for VLA.
 CPPCHECK+=("--suppress=allocaCalled")
-# We actually write C code.
+# False positives in switch statements.
-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")
+# We're a library. This only works on whole programs.
+CPPCHECK_C=("--suppress=unusedFunction")
+
+# We actually write C code.
+CPPCHECK_CXX=("--suppress=cstyleCast")
+# False positive in auto_tests.
+CPPCHECK_CXX+=("--suppress=shadowArgument")
+CPPCHECK_CXX+=("--suppress=shadowFunction")
+# Range-for is fine.
+CPPCHECK_CXX+=("--suppress=useStlAlgorithm")
 
 run() {
   echo "Running cppcheck in variant '$*'"
-  cppcheck "${CPPCHECK[@]}" amalgamation.cc "${CPPFLAGS[@]}" "$@"
+  cppcheck "${CPPCHECK[@]}" "${CPPCHECK_C[@]}" tox*/*.[ch] tox*/*/*.[ch] "${CPPFLAGS[@]}" "$@"
+  cppcheck "${CPPCHECK[@]}" "${CPPCHECK_CXX[@]}" amalgamation.cc "${CPPFLAGS[@]}" "$@"
 }
 
 . other/analysis/variants.sh

other/bootstrap_daemon/docker/tox-bootstrapd.sha256

@@ -1 +1 @@
-73bc2fb0838ba342f92fa5e2c4a85989c61d04fd87a20db7edc300a9bb42781a  /usr/local/bin/tox-bootstrapd
+6a7b0559334d281aacd5ebc18ed5523da1ac49d9dd09d81a33166a598d4a97b5  /usr/local/bin/tox-bootstrapd

toxcore/DHT.c

@@ -464,21 +464,21 @@ int pack_ip_port(const Logger *logger, uint8_t *data, uint16_t length, const IP_
     }
 
     bool is_ipv4;
-    uint8_t net_family;
+    uint8_t family;
 
     if (net_family_is_ipv4(ip_port->ip.family)) {
         // TODO(irungentoo): use functions to convert endianness
         is_ipv4 = true;
-        net_family = TOX_AF_INET;
+        family = TOX_AF_INET;
     } else if (net_family_is_tcp_ipv4(ip_port->ip.family)) {
         is_ipv4 = true;
-        net_family = TOX_TCP_INET;
+        family = TOX_TCP_INET;
     } else if (net_family_is_ipv6(ip_port->ip.family)) {
         is_ipv4 = false;
-        net_family = TOX_AF_INET6;
+        family = TOX_AF_INET6;
     } else if (net_family_is_tcp_ipv6(ip_port->ip.family)) {
         is_ipv4 = false;
-        net_family = TOX_TCP_INET6;
+        family = TOX_TCP_INET6;
     } else {
         char ip_str[IP_NTOA_LEN];
         // TODO(iphydf): Find out why we're trying to pack invalid IPs, stop
@@ -494,7 +494,7 @@ int pack_ip_port(const Logger *logger, uint8_t *data, uint16_t length, const IP_
             return -1;
         }
 
-        data[0] = net_family;
+        data[0] = family;
         memcpy(data + 1, &ip_port->ip.ip.v4, SIZE_IP4);
         memcpy(data + 1 + SIZE_IP4, &ip_port->port, sizeof(uint16_t));
         return size;
@@ -505,7 +505,7 @@ int pack_ip_port(const Logger *logger, uint8_t *data, uint16_t length, const IP_
             return -1;
         }
 
-        data[0] = net_family;
+        data[0] = family;
         memcpy(data + 1, &ip_port->ip.ip.v6, SIZE_IP6);
         memcpy(data + 1 + SIZE_IP6, &ip_port->port, sizeof(uint16_t));
         return size;

toxcore/TCP_client.c

@@ -614,6 +614,142 @@ TCP_Client_Connection *new_TCP_connection(const Logger *logger, const Mono_Time
     return temp;
 }
 
+non_null()
+static int handle_TCP_client_routing_response(TCP_Client_Connection *conn, const uint8_t *data, uint16_t length)
+{
+    if (length != 1 + 1 + CRYPTO_PUBLIC_KEY_SIZE) {
+        return -1;
+    }
+
+    if (data[1] < NUM_RESERVED_PORTS) {
+        return 0;
+    }
+
+    const uint8_t con_id = data[1] - NUM_RESERVED_PORTS;
+
+    if (conn->connections[con_id].status != 0) {
+        return 0;
+    }
+
+    conn->connections[con_id].status = 1;
+    conn->connections[con_id].number = -1;
+    memcpy(conn->connections[con_id].public_key, data + 2, CRYPTO_PUBLIC_KEY_SIZE);
+
+    if (conn->response_callback != nullptr) {
+        conn->response_callback(conn->response_callback_object, con_id, conn->connections[con_id].public_key);
+    }
+
+    return 0;
+}
+
+non_null()
+static int handle_TCP_client_connection_notification(TCP_Client_Connection *conn, const uint8_t *data, uint16_t length)
+{
+    if (length != 1 + 1) {
+        return -1;
+    }
+
+    if (data[1] < NUM_RESERVED_PORTS) {
+        return -1;
+    }
+
+    const uint8_t con_id = data[1] - NUM_RESERVED_PORTS;
+
+    if (conn->connections[con_id].status != 1) {
+        return 0;
+    }
+
+    conn->connections[con_id].status = 2;
+
+    if (conn->status_callback != nullptr) {
+        conn->status_callback(conn->status_callback_object, conn->connections[con_id].number, con_id,
+                              conn->connections[con_id].status);
+    }
+
+    return 0;
+}
+
+non_null()
+static int handle_TCP_client_disconnect_notification(TCP_Client_Connection *conn, const uint8_t *data, uint16_t length)
+{
+    if (length != 1 + 1) {
+        return -1;
+    }
+
+    if (data[1] < NUM_RESERVED_PORTS) {
+        return -1;
+    }
+
+    const uint8_t con_id = data[1] - NUM_RESERVED_PORTS;
+
+    if (conn->connections[con_id].status == 0) {
+        return 0;
+    }
+
+    if (conn->connections[con_id].status != 2) {
+        return 0;
+    }
+
+    conn->connections[con_id].status = 1;
+
+    if (conn->status_callback != nullptr) {
+        conn->status_callback(conn->status_callback_object, conn->connections[con_id].number, con_id,
+                              conn->connections[con_id].status);
+    }
+
+    return 0;
+}
+
+non_null()
+static int handle_TCP_client_ping(const Logger *logger, TCP_Client_Connection *conn, const uint8_t *data, uint16_t length)
+{
+    if (length != 1 + sizeof(uint64_t)) {
+        return -1;
+    }
+
+    uint64_t ping_id;
+    memcpy(&ping_id, data + 1, sizeof(uint64_t));
+    conn->ping_response_id = ping_id;
+    tcp_send_ping_response(logger, conn);
+    return 0;
+}
+
+non_null()
+static int handle_TCP_client_pong(TCP_Client_Connection *conn, const uint8_t *data, uint16_t length)
+{
+    if (length != 1 + sizeof(uint64_t)) {
+        return -1;
+    }
+
+    uint64_t ping_id;
+    memcpy(&ping_id, data + 1, sizeof(uint64_t));
+
+    if (ping_id != 0) {
+        if (ping_id == conn->ping_id) {
+            conn->ping_id = 0;
+        }
+
+        return 0;
+    }
+
+    return -1;
+}
+
+non_null(1, 2) nullable(4)
+static int handle_TCP_client_oob_recv(TCP_Client_Connection *conn, const uint8_t *data, uint16_t length, void *userdata)
+{
+    if (length <= 1 + CRYPTO_PUBLIC_KEY_SIZE) {
+        return -1;
+    }
+
+    if (conn->oob_data_callback != nullptr) {
+        conn->oob_data_callback(conn->oob_data_callback_object, data + 1, data + 1 + CRYPTO_PUBLIC_KEY_SIZE,
+                                length - (1 + CRYPTO_PUBLIC_KEY_SIZE), userdata);
+    }
+
+    return 0;
+}
+
 /**
  * @retval 0 on success
  * @retval -1 on failure
@@ -627,129 +763,23 @@ static int handle_TCP_client_packet(const Logger *logger, TCP_Client_Connection
     }
 
     switch (data[0]) {
-        case TCP_PACKET_ROUTING_RESPONSE: {
+        case TCP_PACKET_ROUTING_RESPONSE:
-            if (length != 1 + 1 + CRYPTO_PUBLIC_KEY_SIZE) {
+            return handle_TCP_client_routing_response(conn, data, length);
-                return -1;
-            }
 
-            if (data[1] < NUM_RESERVED_PORTS) {
+        case TCP_PACKET_CONNECTION_NOTIFICATION:
-                return 0;
+            return handle_TCP_client_connection_notification(conn, data, length);
-            }
 
-            const uint8_t con_id = data[1] - NUM_RESERVED_PORTS;
+        case TCP_PACKET_DISCONNECT_NOTIFICATION:
+            return handle_TCP_client_disconnect_notification(conn, data, length);
 
-            if (conn->connections[con_id].status != 0) {
+        case TCP_PACKET_PING:
-                return 0;
+            return handle_TCP_client_ping(logger, conn, data, length);
-            }
 
-            conn->connections[con_id].status = 1;
+        case TCP_PACKET_PONG:
-            conn->connections[con_id].number = -1;
+            return handle_TCP_client_pong(conn, data, length);
-            memcpy(conn->connections[con_id].public_key, data + 2, CRYPTO_PUBLIC_KEY_SIZE);
 
-            if (conn->response_callback != nullptr) {
+        case TCP_PACKET_OOB_RECV:
-                conn->response_callback(conn->response_callback_object, con_id, conn->connections[con_id].public_key);
+            return handle_TCP_client_oob_recv(conn, data, length, userdata);
-            }
-
-            return 0;
-        }
-
-        case TCP_PACKET_CONNECTION_NOTIFICATION: {
-            if (length != 1 + 1) {
-                return -1;
-            }
-
-            if (data[1] < NUM_RESERVED_PORTS) {
-                return -1;
-            }
-
-            uint8_t con_id = data[1] - NUM_RESERVED_PORTS;
-
-            if (conn->connections[con_id].status != 1) {
-                return 0;
-            }
-
-            conn->connections[con_id].status = 2;
-
-            if (conn->status_callback != nullptr) {
-                conn->status_callback(conn->status_callback_object, conn->connections[con_id].number, con_id,
-                                      conn->connections[con_id].status);
-            }
-
-            return 0;
-        }
-
-        case TCP_PACKET_DISCONNECT_NOTIFICATION: {
-            if (length != 1 + 1) {
-                return -1;
-            }
-
-            if (data[1] < NUM_RESERVED_PORTS) {
-                return -1;
-            }
-
-            uint8_t con_id = data[1] - NUM_RESERVED_PORTS;
-
-            if (conn->connections[con_id].status == 0) {
-                return 0;
-            }
-
-            if (conn->connections[con_id].status != 2) {
-                return 0;
-            }
-
-            conn->connections[con_id].status = 1;
-
-            if (conn->status_callback != nullptr) {
-                conn->status_callback(conn->status_callback_object, conn->connections[con_id].number, con_id,
-                                      conn->connections[con_id].status);
-            }
-
-            return 0;
-        }
-
-        case TCP_PACKET_PING: {
-            if (length != 1 + sizeof(uint64_t)) {
-                return -1;
-            }
-
-            uint64_t ping_id;
-            memcpy(&ping_id, data + 1, sizeof(uint64_t));
-            conn->ping_response_id = ping_id;
-            tcp_send_ping_response(logger, conn);
-            return 0;
-        }
-
-        case TCP_PACKET_PONG: {
-            if (length != 1 + sizeof(uint64_t)) {
-                return -1;
-            }
-
-            uint64_t ping_id;
-            memcpy(&ping_id, data + 1, sizeof(uint64_t));
-
-            if (ping_id != 0) {
-                if (ping_id == conn->ping_id) {
-                    conn->ping_id = 0;
-                }
-
-                return 0;
-            }
-
-            return -1;
-        }
-
-        case TCP_PACKET_OOB_RECV: {
-            if (length <= 1 + CRYPTO_PUBLIC_KEY_SIZE) {
-                return -1;
-            }
-
-            if (conn->oob_data_callback != nullptr) {
-                conn->oob_data_callback(conn->oob_data_callback_object, data + 1, data + 1 + CRYPTO_PUBLIC_KEY_SIZE,
-                                        length - (1 + CRYPTO_PUBLIC_KEY_SIZE), userdata);
-            }
-
-            return 0;
-        }
 
         case TCP_PACKET_ONION_RESPONSE: {
             conn->onion_callback(conn->onion_callback_object, data + 1, length - 1, userdata);
@@ -761,7 +791,7 @@ static int handle_TCP_client_packet(const Logger *logger, TCP_Client_Connection
                 return -1;
             }
 
-            uint8_t con_id = data[0] - NUM_RESERVED_PORTS;
+            const uint8_t con_id = data[0] - NUM_RESERVED_PORTS;
 
             if (conn->data_callback != nullptr) {
                 conn->data_callback(conn->data_callback_object, conn->connections[con_id].number, con_id, data + 1, length - 1,

toxencryptsave/toxencryptsave.c

@@ -234,10 +234,10 @@ bool tox_pass_encrypt(const uint8_t *plaintext, size_t plaintext_len, const uint
  *
  * returns true on success
  */
-bool tox_pass_key_decrypt(const Tox_Pass_Key *key, const uint8_t *ciphertext, size_t length, uint8_t *plaintext,
+bool tox_pass_key_decrypt(const Tox_Pass_Key *key, const uint8_t *ciphertext, size_t ciphertext_len,
-                          Tox_Err_Decryption *error)
+                          uint8_t *plaintext, Tox_Err_Decryption *error)
 {
-    if (length <= TOX_PASS_ENCRYPTION_EXTRA_LENGTH) {
+    if (ciphertext_len <= TOX_PASS_ENCRYPTION_EXTRA_LENGTH) {
         SET_ERROR_PARAMETER(error, TOX_ERR_DECRYPTION_INVALID_LENGTH);
         return false;
     }
@@ -255,7 +255,7 @@ bool tox_pass_key_decrypt(const Tox_Pass_Key *key, const uint8_t *ciphertext, si
     ciphertext += TOX_ENC_SAVE_MAGIC_LENGTH;
     ciphertext += crypto_pwhash_scryptsalsa208sha256_SALTBYTES; // salt only affects key derivation
 
-    const size_t decrypt_length = length - TOX_PASS_ENCRYPTION_EXTRA_LENGTH;
+    const size_t decrypt_length = ciphertext_len - TOX_PASS_ENCRYPTION_EXTRA_LENGTH;
 
     uint8_t nonce[crypto_box_NONCEBYTES];
     memcpy(nonce, ciphertext, crypto_box_NONCEBYTES);