refactor: Add nullable annotations to struct members.

Also add a bunch of casts where needed. I've tried to model everything
in such a way that it minimises casts. The casts *should* be safe, but
it's not always obvious. In the obvious cases, we should have a linter
that validates it. In the non-obvious cases, that linter should warn and
require that we add a null check. I've added some null checks in some
cases but not all.

Also, refactored some of the constructor functions to never assign a
maybe-null value to a non-null struct member, instead using a temporary
local variable to check if construction/allocation succeeded.

Author: iphydf

other/analysis/run-cppcheck

@@ -16,6 +16,7 @@ CPPCHECK+=("--error-exitcode=1")
 CPPCHECK+=("--suppress=unmatchedSuppression")
 # We don't cast function pointers, which cppcheck suggests here.
 CPPCHECK+=("--suppress=constParameterCallback")
+CPPCHECK+=("--suppress=constParameterPointer")
 # This disagrees with clang's warnings.
 CPPCHECK+=("--suppress=invalidPrintfArgType_uint")
 # False positives in switch statements.

testing/Messenger_test.c

@@ -103,10 +103,10 @@ int main(int argc, char *argv[])
     Messenger_Options options = {0};
     options.ipv6enabled = ipv6enabled;
     Messenger_Error err;
-    m = new_messenger(mono_time, mem, os_random(), os_network(), &options, &err);
+    m = new_messenger(mono_time, mem, (const Random * _Nonnull)os_random(), (const Network * _Nonnull)os_network(), &options, &err);
 
     if (!m) {
-        fprintf(stderr, "Failed to allocate messenger datastructure: %d\n", err);
+        fprintf(stderr, "Failed to allocate messenger datastructure: %u\n", err);
         exit(0);
     }
 

testing/fuzzing/protodump.cc

@@ -188,7 +188,7 @@ void RecordBootstrap(const char *init, const char *bootstrap)
         [](Tox *tox, Tox_Log_Level level, const char *file, uint32_t line, const char *func,
             const char *message, void *user_data) {
             // Log to stdout.
-            std::printf("[%s] %c %s:%d(%s): %s\n", static_cast<Record_System *>(user_data)->name_,
+            std::printf("[%s] %c %s:%u(%s): %s\n", static_cast<Record_System *>(user_data)->name_,
                 tox_log_level_name(level), file, line, func, message);
         });
 

testing/fuzzing/protodump_reduce.cc

@@ -152,7 +152,7 @@ void TestEndToEnd(Fuzz_Data &input)
             const char *message, void *user_data) {
             // Log to stdout.
             if (PROTODUMP_DEBUG) {
-                std::printf("[tox1] %c %s:%d(%s): %s\n", tox_log_level_name(level), file, line,
+                std::printf("[tox1] %c %s:%u(%s): %s\n", tox_log_level_name(level), file, line,
                     func, message);
             }
         });

toxcore/DHT.c

@@ -57,8 +57,8 @@
 #define KEYS_TIMEOUT 600
 
 typedef struct DHT_Friend_Callback {
-    dht_ip_cb *ip_callback;
-    void *data;
+    dht_ip_cb *_Nullable ip_callback;
+    void *_Nullable data;
     int32_t number;
 } DHT_Friend_Callback;
 
@@ -88,17 +88,17 @@ const Node_format empty_node_format = {{0}};
 static_assert(sizeof(empty_dht_friend.lock_flags) * 8 == DHT_FRIEND_MAX_LOCKS, "Bitfield size and number of locks don't match");
 
 typedef struct Cryptopacket_Handler {
-    cryptopacket_handler_cb *function;
-    void *object;
+    cryptopacket_handler_cb *_Nullable function;
+    void *_Nullable object;
 } Cryptopacket_Handler;
 
 struct DHT {
-    const Logger *log;
-    const Network *ns;
-    Mono_Time *mono_time;
-    const Memory *mem;
-    const Random *rng;
-    Networking_Core *net;
+    const Logger *_Nonnull log;
+    const Network *_Nonnull ns;
+    Mono_Time *_Nonnull mono_time;
+    const Memory *_Nonnull mem;
+    const Random *_Nonnull rng;
+    Networking_Core *_Nonnull net;
 
     bool hole_punching_enabled;
     bool lan_discovery_enabled;
@@ -111,26 +111,26 @@ struct DHT {
     uint8_t self_public_key[CRYPTO_PUBLIC_KEY_SIZE];
     uint8_t self_secret_key[CRYPTO_SECRET_KEY_SIZE];
 
-    DHT_Friend    *friends_list;
+    DHT_Friend    *_Nullable friends_list;
     uint16_t       num_friends;
 
-    Node_format   *loaded_nodes_list;
+    Node_format   *_Nullable loaded_nodes_list;
     uint32_t       loaded_num_nodes;
     unsigned int   loaded_nodes_index;
 
-    Shared_Key_Cache *shared_keys_recv;
-    Shared_Key_Cache *shared_keys_sent;
+    Shared_Key_Cache *_Nonnull shared_keys_recv;
+    Shared_Key_Cache *_Nonnull shared_keys_sent;
 
-    struct Ping   *ping;
-    Ping_Array    *dht_ping_array;
+    struct Ping   *_Nonnull ping;
+    Ping_Array    *_Nonnull dht_ping_array;
     uint64_t       cur_time;
 
     Cryptopacket_Handler cryptopackethandlers[256];
 
     Node_format to_bootstrap[MAX_CLOSE_TO_BOOTSTRAP_NODES];
     unsigned int num_to_bootstrap;
 
-    dht_nodes_response_cb *nodes_response_callback;
+    dht_nodes_response_cb *_Nullable nodes_response_callback;
 };
 
 const uint8_t *dht_friend_public_key(const DHT_Friend *dht_friend)
@@ -734,7 +734,7 @@ int get_close_nodes(
     return get_somewhat_close_nodes(
                dht->cur_time, public_key, nodes_list,
                sa_family, dht->close_clientlist,
-               dht->friends_list, dht->num_friends,
+               (const DHT_Friend * _Nonnull)dht->friends_list, dht->num_friends,
                is_lan, want_announce);
 }
 
@@ -849,9 +849,9 @@ static bool store_node_ok(const Client_data *_Nonnull client, uint64_t cur_time,
 }
 
 typedef struct Client_data_Cmp {
-    const Memory *mem;
+    const Memory *_Nonnull mem;
     uint64_t cur_time;
-    const uint8_t *comp_public_key;
+    const uint8_t *_Nonnull comp_public_key;
 } Client_data_Cmp;
 
 static int client_data_cmp(const Client_data_Cmp *_Nonnull cmp, const Client_data *_Nonnull entry1, const Client_data *_Nonnull entry2)
@@ -2509,14 +2509,16 @@ DHT *new_dht(const Logger *log, const Memory *mem, const Random *rng, const Netw
     dht->hole_punching_enabled = hole_punching_enabled;
     dht->lan_discovery_enabled = lan_discovery_enabled;
 
-    dht->ping = ping_new(mem, mono_time, rng, dht);
+    struct Ping *temp_ping = ping_new(mem, mono_time, rng, dht);
 
-    if (dht->ping == nullptr) {
+    if (temp_ping == nullptr) {
         LOGGER_ERROR(log, "failed to initialise ping");
         kill_dht(dht);
         return nullptr;
     }
 
+    dht->ping = temp_ping;
+
     networking_registerhandler(dht->net, NET_PACKET_NODES_REQUEST, &handle_nodes_request, dht);
     networking_registerhandler(dht->net, NET_PACKET_NODES_RESPONSE, &handle_nodes_response, dht);
     networking_registerhandler(dht->net, NET_PACKET_CRYPTO, &cryptopacket_handle, dht);
@@ -2529,23 +2531,36 @@ DHT *new_dht(const Logger *log, const Memory *mem, const Random *rng, const Netw
 
     crypto_new_keypair(rng, dht->self_public_key, dht->self_secret_key);
 
-    dht->shared_keys_recv = shared_key_cache_new(log, mono_time, mem, dht->self_secret_key, KEYS_TIMEOUT, MAX_KEYS_PER_SLOT);
-    dht->shared_keys_sent = shared_key_cache_new(log, mono_time, mem, dht->self_secret_key, KEYS_TIMEOUT, MAX_KEYS_PER_SLOT);
+    Shared_Key_Cache *const temp_shared_keys_recv = shared_key_cache_new(log, mono_time, mem, dht->self_secret_key, KEYS_TIMEOUT, MAX_KEYS_PER_SLOT);
+
+    if (temp_shared_keys_recv == nullptr) {
+        LOGGER_ERROR(log, "failed to initialise shared key cache");
+        kill_dht(dht);
+        return nullptr;
+    }
+
+    dht->shared_keys_recv = temp_shared_keys_recv;
+
+    Shared_Key_Cache *const temp_shared_keys_sent = shared_key_cache_new(log, mono_time, mem, dht->self_secret_key, KEYS_TIMEOUT, MAX_KEYS_PER_SLOT);
 
-    if (dht->shared_keys_recv == nullptr || dht->shared_keys_sent == nullptr) {
+    if (temp_shared_keys_sent == nullptr) {
         LOGGER_ERROR(log, "failed to initialise shared key cache");
         kill_dht(dht);
         return nullptr;
     }
 
-    dht->dht_ping_array = ping_array_new(mem, DHT_PING_ARRAY_SIZE, PING_TIMEOUT);
+    dht->shared_keys_sent = temp_shared_keys_sent;
 
-    if (dht->dht_ping_array == nullptr) {
+    Ping_Array *const temp_ping_array = ping_array_new(mem, DHT_PING_ARRAY_SIZE, PING_TIMEOUT);
+
+    if (temp_ping_array == nullptr) {
         LOGGER_ERROR(log, "failed to initialise ping array");
         kill_dht(dht);
         return nullptr;
     }
 
+    dht->dht_ping_array = temp_ping_array;
+
     for (uint32_t i = 0; i < DHT_FAKE_FRIEND_NUMBER; ++i) {
         uint8_t random_public_key_bytes[CRYPTO_PUBLIC_KEY_SIZE];
         uint8_t random_secret_key_bytes[CRYPTO_SECRET_KEY_SIZE];
@@ -2600,7 +2615,7 @@ void kill_dht(DHT *dht)
     networking_registerhandler(dht->net, NET_PACKET_NODES_RESPONSE, nullptr, nullptr);
     networking_registerhandler(dht->net, NET_PACKET_CRYPTO, nullptr, nullptr);
     networking_registerhandler(dht->net, NET_PACKET_LAN_DISCOVERY, nullptr, nullptr);
-    cryptopacket_registerhandler(dht, CRYPTO_PACKET_NAT_PING, nullptr, nullptr);
+    cryptopacket_registerhandler((DHT * _Nonnull)dht, CRYPTO_PACKET_NAT_PING, nullptr, nullptr);
 
     shared_key_cache_free(dht->shared_keys_recv);
     shared_key_cache_free(dht->shared_keys_sent);

toxcore/DHT.h

@@ -212,7 +212,7 @@ int unpack_nodes(Node_format *_Nonnull nodes, uint16_t max_num_nodes, uint16_t *
                  uint16_t length, bool tcp_enabled);
 /*----------------------------------------------------------------------------------*/
 
-typedef int cryptopacket_handler_cb(void *_Nonnull object, const IP_Port *_Nonnull source, const uint8_t *_Nonnull source_pubkey,
+typedef int cryptopacket_handler_cb(void *_Nullable object, const IP_Port *_Nonnull source, const uint8_t *_Nonnull source_pubkey,
                                     const uint8_t *_Nonnull packet, uint16_t length, void *_Nullable userdata);
 
 typedef struct DHT DHT;