cleanup: Upgrade to clang-tidy-17 and fix some warnings.

Author: iphydf

.circleci/config.yml

@@ -15,7 +15,6 @@ workflows:
       - ubsan
       # Static analysis
       - clang-analyze
-      - clang-tidy
       - cpplint
       - infer
       - static-analysis
@@ -154,24 +153,6 @@ jobs:
       - run: git submodule update --init --recursive
       - run: other/analysis/run-clang-analyze
 
-  clang-tidy:
-    working_directory: ~/work
-    docker:
-      - image: ubuntu
-
-    steps:
-      - run: *apt_install
-      - run:
-          apt-get install -y --no-install-recommends
-            ca-certificates
-            clang-tidy-14
-      - checkout
-      - run: git submodule update --init --recursive
-      - run:
-          other/analysis/run-clang-tidy ||
-          other/analysis/run-clang-tidy ||
-          other/analysis/run-clang-tidy
-
   cpplint:
     working_directory: ~/work
     docker:

.clang-tidy

@@ -35,6 +35,8 @@ CheckOptions:
     value:           lower_case
 
   - key:             llvmlibc-restrict-system-libc-headers.Includes
-    value:           "arpa/inet.h,assert.h,ctype.h,errno.h,fcntl.h,getopt.h,libconfig.h,linux/netdevice.h,math.h,netdb.h,netinet/in.h,opus.h,pthread.h,signal.h,sodium/crypto_scalarmult_curve25519.h,sodium.h,sodium/randombytes.h,stdio.h,stdlib.h,string.h,sys/ioctl.h,syslog.h,sys/resource.h,sys/socket.h,sys/stat.h,sys/time.h,sys/types.h,time.h,unistd.h,vpx/vp8cx.h,vpx/vp8dx.h,vpx/vpx_decoder.h,vpx/vpx_encoder.h,vpx/vpx_image.h"
+    value:           "arpa/inet.h,assert.h,ctype.h,errno.h,fcntl.h,getopt.h,libconfig.h,limits.h,linux/netdevice.h,math.h,netdb.h,netinet/in.h,opus.h,pthread.h,signal.h,sodium/crypto_scalarmult_curve25519.h,sodium.h,sodium/randombytes.h,stdarg.h,stdbool.h,stdint.h,stdio.h,stdlib.h,string.h,sys/ioctl.h,syslog.h,sys/resource.h,sys/socket.h,sys/stat.h,sys/time.h,sys/types.h,time.h,unistd.h,vpx/vp8cx.h,vpx/vp8dx.h,vpx/vpx_decoder.h,vpx/vpx_encoder.h,vpx/vpx_image.h"
   - key:             hicpp-signed-bitwise.IgnorePositiveIntegerLiterals
     value:           true
+  - key:             concurrency-mt-unsafe.FunctionSet
+    value:           posix

.github/workflows/ci.yml

@@ -40,6 +40,16 @@ jobs:
           (find . -name "*.py" -and -not -name "conanfile.py"; grep -lR '^#!.*python') \
               | xargs -n1 -P8 mypy --strict
 
+  clang-tidy:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Set up Docker Buildx
+        uses: docker/setup-buildx-action@v3
+      - name: Docker Build
+        uses: docker/build-push-action@v4
+        with:
+          file: other/docker/clang-tidy/Dockerfile
+
   doxygen:
     runs-on: ubuntu-latest
     steps:

other/DHT_bootstrap.c

@@ -46,7 +46,7 @@ static const char *motd_str = ""; //Change this to anything within 256 bytes(but
 #define PORT 33445
 
 
-static void manage_keys(DHT *dht)
+static bool manage_keys(DHT *dht)
 {
     enum { KEYS_SIZE = CRYPTO_PUBLIC_KEY_SIZE + CRYPTO_SECRET_KEY_SIZE };
     uint8_t keys[KEYS_SIZE];
@@ -60,7 +60,8 @@ static void manage_keys(DHT *dht)
 
         if (read_size != KEYS_SIZE) {
             printf("Error while reading the key file\nExiting.\n");
-            exit(1);
+            fclose(keys_file);
+            return false;
         }
 
         dht_set_self_public_key(dht, keys);
@@ -73,18 +74,20 @@ static void manage_keys(DHT *dht)
 
         if (keys_file == nullptr) {
             printf("Error opening key file in write mode.\nKeys will not be saved.\n");
-            return;
+            return false;
         }
 
         if (fwrite(keys, sizeof(uint8_t), KEYS_SIZE, keys_file) != KEYS_SIZE) {
             printf("Error while writing the key file.\nExiting.\n");
-            exit(1);
+            fclose(keys_file);
+            return false;
         }
 
         printf("Keys saved successfully.\n");
     }
 
     fclose(keys_file);
+    return true;
 }
 
 static const char *strlevel(Logger_Level level)
@@ -121,15 +124,15 @@ int main(int argc, char *argv[])
     if (argc == 2 && !tox_strncasecmp(argv[1], "-h", 3)) {
         printf("Usage (connected)  : %s [--ipv4|--ipv6] IP PORT KEY\n", argv[0]);
         printf("Usage (unconnected): %s [--ipv4|--ipv6]\n", argv[0]);
-        exit(0);
+        return 0;
     }
 
     /* let user override default by cmdline */
     bool ipv6enabled = TOX_ENABLE_IPV6_DEFAULT; /* x */
     int argvoffset = cmdline_parsefor_ipv46(argc, argv, &ipv6enabled);
 
     if (argvoffset < 0) {
-        exit(1);
+        return 1;
     }
 
     /* Initialize networking -
@@ -162,14 +165,16 @@ int main(int argc, char *argv[])
 
     if (!(onion && forwarding && onion_a)) {
         printf("Something failed to initialize.\n");
-        exit(1);
+        return 1;
     }
 
     gca_onion_init(gc_announces_list, onion_a);
 
     perror("Initialization");
 
-    manage_keys(dht);
+    if (!manage_keys(dht)) {
+        return 1;
+    }
     printf("Public key: ");
 
 #ifdef TCP_RELAY_ENABLED
@@ -179,7 +184,7 @@ int main(int argc, char *argv[])
 
     if (tcp_s == nullptr) {
         printf("TCP server failed to initialize.\n");
-        exit(1);
+        return 1;
     }
 
 #endif
@@ -189,7 +194,7 @@ int main(int argc, char *argv[])
 
     if (file == nullptr) {
         printf("Could not open file \"%s\" for writing. Exiting...\n", public_id_filename);
-        exit(1);
+        return 1;
     }
 
     for (uint32_t i = 0; i < 32; ++i) {
@@ -210,7 +215,7 @@ int main(int argc, char *argv[])
 
         if (port_conv <= 0 || port_conv > UINT16_MAX) {
             printf("Failed to convert \"%s\" into a valid port. Exiting...\n", argv[argvoffset + 2]);
-            exit(1);
+            return 1;
         }
 
         const uint16_t port = net_htons((uint16_t)port_conv);
@@ -222,7 +227,7 @@ int main(int argc, char *argv[])
 
         if (!res) {
             printf("Failed to convert \"%s\" into an IP address. Exiting...\n", argv[argvoffset + 1]);
-            exit(1);
+            return 1;
         }
     }
 

other/analysis/run-clang-tidy

@@ -1,6 +1,55 @@
-#!/bin/bash
+#!/usr/bin/env bash
 
 CHECKS="*"
+ERRORS="*"
+
+# Need to investigate or disable and document these.
+# =========================================================
+
+# TODO(iphydf): Fix these.
+ERRORS="$ERRORS,-cert-err34-c"
+ERRORS="$ERRORS,-cert-str34-c"
+ERRORS="$ERRORS,-readability-suspicious-call-argument"
+
+# TODO(iphydf): Fix these.
+CHECKS="$CHECKS,-bugprone-switch-missing-default-case"
+CHECKS="$CHECKS,-misc-include-cleaner"
+
+# TODO(iphydf): We might want some of these. For the ones we don't want, add a
+# comment explaining why not.
+CHECKS="$CHECKS,-clang-analyzer-optin.performance.Padding"
+CHECKS="$CHECKS,-hicpp-signed-bitwise"
+CHECKS="$CHECKS,-readability-function-cognitive-complexity"
+
+# TODO(iphydf): Maybe fix these?
+CHECKS="$CHECKS,-bugprone-easily-swappable-parameters"
+CHECKS="$CHECKS,-bugprone-implicit-widening-of-multiplication-result"
+CHECKS="$CHECKS,-bugprone-integer-division"
+CHECKS="$CHECKS,-clang-analyzer-core.NullDereference"
+CHECKS="$CHECKS,-clang-analyzer-valist.Uninitialized"
+CHECKS="$CHECKS,-cppcoreguidelines-avoid-non-const-global-variables"
+CHECKS="$CHECKS,-misc-no-recursion"
+
+# TODO(iphydf): Probably fix these.
+CHECKS="$CHECKS,-cert-err33-c"
+CHECKS="$CHECKS,-cppcoreguidelines-avoid-magic-numbers"
+CHECKS="$CHECKS,-google-readability-casting"
+CHECKS="$CHECKS,-modernize-macro-to-enum"
+CHECKS="$CHECKS,-readability-magic-numbers"
+
+# Documented disabled checks. We don't want these for sure.
+# =========================================================
+
+# Callback handlers often don't use all their parameters. There's
+# IgnoreVirtual, but that doesn't work for C-style callbacks.
+CHECKS="$CHECKS,-misc-unused-parameters"
+
+# We sometimes use #if 0.
+CHECKS="$CHECKS,-readability-avoid-unconditional-preprocessor-if"
+
+# We have better macro hygiene checks with tokstyle. We can never pass macro
+# arguments that require parentheses inside the macro.
+CHECKS="$CHECKS,-bugprone-macro-parentheses"
 
 # We don't use memcpy_s.
 CHECKS="$CHECKS,-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling"
@@ -81,42 +130,6 @@ CHECKS="$CHECKS,-cert-dcl03-c"
 CHECKS="$CHECKS,-hicpp-static-assert"
 CHECKS="$CHECKS,-misc-static-assert"
 
-# TODO(iphydf): We might want some of these. For the ones we don't want, add a
-# comment explaining why not.
-CHECKS="$CHECKS,-clang-analyzer-optin.performance.Padding"
-CHECKS="$CHECKS,-hicpp-signed-bitwise"
-CHECKS="$CHECKS,-misc-unused-parameters"
-CHECKS="$CHECKS,-readability-function-cognitive-complexity"
-
-# TODO(iphydf): Maybe fix these?
-CHECKS="$CHECKS,-bugprone-easily-swappable-parameters"
-CHECKS="$CHECKS,-bugprone-implicit-widening-of-multiplication-result"
-CHECKS="$CHECKS,-bugprone-integer-division"
-CHECKS="$CHECKS,-clang-analyzer-core.NullDereference"
-CHECKS="$CHECKS,-clang-analyzer-valist.Uninitialized"
-CHECKS="$CHECKS,-concurrency-mt-unsafe"
-CHECKS="$CHECKS,-cppcoreguidelines-avoid-non-const-global-variables"
-CHECKS="$CHECKS,-misc-no-recursion"
-
-# TODO(iphydf): Probably fix these.
-CHECKS="$CHECKS,-cert-err33-c"
-CHECKS="$CHECKS,-cppcoreguidelines-avoid-magic-numbers"
-CHECKS="$CHECKS,-google-readability-casting"
-CHECKS="$CHECKS,-modernize-macro-to-enum"
-CHECKS="$CHECKS,-readability-magic-numbers"
-
-# TODO(iphydf): These two trip on list.c. Investigate why.
-CHECKS="$CHECKS,-clang-analyzer-core.NonNullParamChecker"
-CHECKS="$CHECKS,-clang-analyzer-unix.Malloc"
-
-ERRORS="*"
-
-# TODO(iphydf): Fix these.
-ERRORS="$ERRORS,-bugprone-macro-parentheses"
-ERRORS="$ERRORS,-cert-err34-c"
-ERRORS="$ERRORS,-cert-str34-c"
-ERRORS="$ERRORS,-readability-suspicious-call-argument"
-
 set -eux
 
 run() {
@@ -125,18 +138,21 @@ run() {
   for i in "${!EXTRA_ARGS[@]}"; do
     EXTRA_ARGS[$i]="--extra-arg=${EXTRA_ARGS[$i]}"
   done
-  clang-tidy-14 \
-    -p=_build \
-    --extra-arg=-DMIN_LOGGER_LEVEL=LOGGER_LEVEL_TRACE \
-    "${EXTRA_ARGS[@]}" \
-    --checks="$CHECKS" \
-    --warnings-as-errors="$ERRORS" \
-    --use-color \
-    other/bootstrap_daemon/src/*.c \
-    other/*.c \
-    toxav/*.c \
-    toxcore/*.c \
-    toxencryptsave/*.c
+  find \
+    other/bootstrap_daemon/src \
+    other \
+    toxav \
+    toxcore \
+    toxcore/events \
+    toxencryptsave \
+    -maxdepth 1 -name "*.c" -print0 \
+    | xargs -0 -n15 -P"$(nproc)" clang-tidy \
+      -p=_build \
+      --extra-arg=-DMIN_LOGGER_LEVEL=LOGGER_LEVEL_TRACE \
+      "${EXTRA_ARGS[@]}" \
+      --checks="$CHECKS" \
+      --warnings-as-errors="$ERRORS" \
+      --use-color
 }
 
 cmake . -B_build -GNinja -DCMAKE_EXPORT_COMPILE_COMMANDS=ON

other/bootstrap_daemon/docker/tox-bootstrapd.sha256

@@ -1 +1 @@
-68432689967d06dd144e5cdfe37751ccc62b2aa85b73a9cc55aff3732dc47fde  /usr/local/bin/tox-bootstrapd
+793cce1916b0d406b8e337d1a5243dc71b07727974cc83a3ef39b7af6cbf6cdb  /usr/local/bin/tox-bootstrapd

other/bootstrap_daemon/src/command_line_arguments.c

@@ -47,13 +47,14 @@ static void print_help(void)
               "  --version              Print version information.\n");
 }
 
-void handle_command_line_arguments(int argc, char *argv[], char **cfg_file_path, LOG_BACKEND *log_backend,
-                                   bool *run_in_foreground)
+Cli_Status handle_command_line_arguments(
+        int argc, char *argv[], char **cfg_file_path, LOG_BACKEND *log_backend,
+        bool *run_in_foreground)
 {
     if (argc < 2) {
         log_write(LOG_LEVEL_ERROR, "Error: No arguments provided.\n\n");
         print_help();
-        exit(1);
+        return CLI_STATUS_ERROR;
     }
 
     opterr = 0;
@@ -89,7 +90,7 @@ void handle_command_line_arguments(int argc, char *argv[], char **cfg_file_path,
 
             case 'h':
                 print_help();
-                exit(0);
+                return CLI_STATUS_DONE;
 
             case 'l':
                 if (strcmp(optarg, "syslog") == 0) {
@@ -101,24 +102,24 @@ void handle_command_line_arguments(int argc, char *argv[], char **cfg_file_path,
                 } else {
                     log_write(LOG_LEVEL_ERROR, "Error: Invalid BACKEND value for --log-backend option passed: %s\n\n", optarg);
                     print_help();
-                    exit(1);
+                    return CLI_STATUS_ERROR;
                 }
 
                 break;
 
             case 'v':
                 log_write(LOG_LEVEL_INFO, "Version: %lu\n", DAEMON_VERSION_NUMBER);
-                exit(0);
+                return CLI_STATUS_DONE;
 
             case '?':
                 log_write(LOG_LEVEL_ERROR, "Error: Unrecognized option %s\n\n", argv[optind - 1]);
                 print_help();
-                exit(1);
+                return CLI_STATUS_ERROR;
 
             case ':':
                 log_write(LOG_LEVEL_ERROR, "Error: No argument provided for option %s\n\n", argv[optind - 1]);
                 print_help();
-                exit(1);
+                return CLI_STATUS_ERROR;
         }
     }
 
@@ -129,6 +130,8 @@ void handle_command_line_arguments(int argc, char *argv[], char **cfg_file_path,
     if (!cfg_file_path_set) {
         log_write(LOG_LEVEL_ERROR, "Error: The required --config option wasn't specified\n\n");
         print_help();
-        exit(1);
+        return CLI_STATUS_ERROR;
     }
+
+    return CLI_STATUS_OK;
 }