From 9df963d0521c48baebb979e74f46c4cb7725c0ab Mon Sep 17 00:00:00 2001 From: iphydf Date: Thu, 31 Aug 2023 14:21:36 +0000 Subject: [PATCH] cleanup: Fix a few more clang-tidy warnings. --- .clang-tidy | 2 ++ other/analysis/run-clang-tidy | 27 ++++++++++--------- .../docker/tox-bootstrapd.sha256 | 2 +- other/bootstrap_daemon/src/tox-bootstrapd.c | 4 --- toxcore/DHT.c | 10 ++----- toxcore/mono_time.c | 2 +- toxcore/net_crypto.c | 5 ++-- toxcore/tox.c | 16 ++++++----- 8 files changed, 33 insertions(+), 35 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 2a396a2cfe..5ad92a14d2 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -36,3 +36,5 @@ CheckOptions: - 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" + - key: hicpp-signed-bitwise.IgnorePositiveIntegerLiterals + value: true diff --git a/other/analysis/run-clang-tidy b/other/analysis/run-clang-tidy index d57a02efbc..46f11fce4b 100755 --- a/other/analysis/run-clang-tidy +++ b/other/analysis/run-clang-tidy @@ -12,7 +12,9 @@ CHECKS="$CHECKS,-clang-diagnostic-unknown-warning-option" # readability issue. CHECKS="$CHECKS,-readability-identifier-length" -# This finds all the do {} while (0) loops and tells us to unroll them. +# Altera checks are for GPUs (OpenCL). Our code doesn't run on GPUs. +CHECKS="$CHECKS,-altera-id-dependent-backward-branch" +CHECKS="$CHECKS,-altera-struct-pack-align" CHECKS="$CHECKS,-altera-unroll-loops" # We target systems other than Android, so we don't want to use non-standard @@ -41,6 +43,14 @@ CHECKS="$CHECKS,-readability-else-after-return" # functions otherwise. CHECKS="$CHECKS,-readability-redundant-control-flow" +# These are incredibly annoying, because things like +# uint16_t a = 0, b = 1, c = a > b ? a : b; +# ^ +# Trip the checker, which is true, because of integer promotion, but also not +# very helpful as a diagnostic. +CHECKS="$CHECKS,-bugprone-narrowing-conversions" +CHECKS="$CHECKS,-cppcoreguidelines-narrowing-conversions" + # 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" @@ -49,21 +59,13 @@ CHECKS="$CHECKS,-misc-unused-parameters" CHECKS="$CHECKS,-readability-function-cognitive-complexity" # TODO(iphydf): Maybe fix these? -CHECKS="$CHECKS,-altera-id-dependent-backward-branch" -CHECKS="$CHECKS,-altera-struct-pack-align" -CHECKS="$CHECKS,-bugprone-branch-clone" CHECKS="$CHECKS,-bugprone-easily-swappable-parameters" CHECKS="$CHECKS,-bugprone-implicit-widening-of-multiplication-result" CHECKS="$CHECKS,-bugprone-integer-division" -CHECKS="$CHECKS,-bugprone-narrowing-conversions" -CHECKS="$CHECKS,-clang-analyzer-core.NonNullParamChecker" CHECKS="$CHECKS,-clang-analyzer-core.NullDereference" -CHECKS="$CHECKS,-clang-analyzer-optin.portability.UnixAPI" -CHECKS="$CHECKS,-clang-analyzer-unix.Malloc" CHECKS="$CHECKS,-clang-analyzer-valist.Uninitialized" CHECKS="$CHECKS,-concurrency-mt-unsafe" CHECKS="$CHECKS,-cppcoreguidelines-avoid-non-const-global-variables" -CHECKS="$CHECKS,-cppcoreguidelines-narrowing-conversions" CHECKS="$CHECKS,-misc-no-recursion" # TODO(iphydf): Probably fix these. @@ -73,15 +75,16 @@ 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,-bugprone-posix-return" -ERRORS="$ERRORS,-bugprone-signed-char-misuse" ERRORS="$ERRORS,-cert-err34-c" ERRORS="$ERRORS,-cert-str34-c" -ERRORS="$ERRORS,-hicpp-uppercase-literal-suffix" ERRORS="$ERRORS,-readability-suspicious-call-argument" set -eux diff --git a/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 b/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 index 71bc96b045..f13c30ca59 100644 --- a/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 +++ b/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 @@ -1 +1 @@ -6f4ec4239de5ea05ae341271bbda4cb78535f943a693321648ee1ab043a203d2 /usr/local/bin/tox-bootstrapd +dd1d4bd8f7eeb5de6a7e6137824d337c9edf6579a24e43442e5fb417add69e07 /usr/local/bin/tox-bootstrapd diff --git a/other/bootstrap_daemon/src/tox-bootstrapd.c b/other/bootstrap_daemon/src/tox-bootstrapd.c index 17802d8739..137d8bcb6b 100644 --- a/other/bootstrap_daemon/src/tox-bootstrapd.c +++ b/other/bootstrap_daemon/src/tox-bootstrapd.c @@ -179,11 +179,7 @@ static LOG_LEVEL logger_level_to_log_level(Logger_Level level) { switch (level) { case LOGGER_LEVEL_TRACE: - return LOG_LEVEL_INFO; - case LOGGER_LEVEL_DEBUG: - return LOG_LEVEL_INFO; - case LOGGER_LEVEL_INFO: return LOG_LEVEL_INFO; diff --git a/toxcore/DHT.c b/toxcore/DHT.c index d717ff5ace..4c8e8353fa 100644 --- a/toxcore/DHT.c +++ b/toxcore/DHT.c @@ -778,11 +778,7 @@ static void get_close_nodes_inner(uint64_t cur_time, const uint8_t *public_key, const IPPTsPng *ipptp = NULL; - if (net_family_is_ipv4(sa_family)) { - ipptp = &client->assoc4; - } else if (net_family_is_ipv6(sa_family)) { - ipptp = &client->assoc6; - } else if (client->assoc4.timestamp >= client->assoc6.timestamp) { + if (net_family_is_ipv4(sa_family) || client->assoc4.timestamp >= client->assoc6.timestamp) { ipptp = &client->assoc4; } else { ipptp = &client->assoc6; @@ -2466,9 +2462,7 @@ static uint16_t list_nodes(const Random *rng, const Client_data *list, size_t le } if (!assoc_timeout(cur_time, &list[i - 1].assoc6)) { - if (assoc == nullptr) { - assoc = &list[i - 1].assoc6; - } else if ((random_u08(rng) % 2) != 0) { + if (assoc == nullptr || (random_u08(rng) % 2) != 0) { assoc = &list[i - 1].assoc6; } } diff --git a/toxcore/mono_time.c b/toxcore/mono_time.c index e03046f799..418deec494 100644 --- a/toxcore/mono_time.c +++ b/toxcore/mono_time.c @@ -138,7 +138,7 @@ Mono_Time *mono_time_new(const Memory *mem, mono_time_current_time_cb *current_t return nullptr; } - if (pthread_rwlock_init(mono_time->time_update_lock, nullptr) < 0) { + if (pthread_rwlock_init(mono_time->time_update_lock, nullptr) != 0) { mem_delete(mem, mono_time->time_update_lock); mem_delete(mem, mono_time); return nullptr; diff --git a/toxcore/net_crypto.c b/toxcore/net_crypto.c index 198631c217..82f8e6c13e 100644 --- a/toxcore/net_crypto.c +++ b/toxcore/net_crypto.c @@ -3066,9 +3066,8 @@ bool crypto_connection_status(const Net_Crypto *c, int crypt_connection_id, bool const uint64_t current_time = mono_time_get(c->mono_time); - if ((UDP_DIRECT_TIMEOUT + conn->direct_lastrecv_timev4) > current_time) { - *direct_connected = true; - } else if ((UDP_DIRECT_TIMEOUT + conn->direct_lastrecv_timev6) > current_time) { + if ((UDP_DIRECT_TIMEOUT + conn->direct_lastrecv_timev4) > current_time || + (UDP_DIRECT_TIMEOUT + conn->direct_lastrecv_timev6) > current_time) { *direct_connected = true; } } diff --git a/toxcore/tox.c b/toxcore/tox.c index 4ee5888483..4377c3c8de 100644 --- a/toxcore/tox.c +++ b/toxcore/tox.c @@ -812,12 +812,16 @@ Tox *tox_new(const struct Tox_Options *options, Tox_Err_New *error) tox->m = new_messenger(tox->mono_time, tox->sys.mem, tox->sys.rng, tox->sys.ns, &m_options, &m_error); if (tox->m == nullptr) { - if (m_error == MESSENGER_ERROR_PORT) { - SET_ERROR_PARAMETER(error, TOX_ERR_NEW_PORT_ALLOC); - } else if (m_error == MESSENGER_ERROR_TCP_SERVER) { - SET_ERROR_PARAMETER(error, TOX_ERR_NEW_PORT_ALLOC); - } else { - SET_ERROR_PARAMETER(error, TOX_ERR_NEW_MALLOC); + switch (m_error) { + case MESSENGER_ERROR_PORT: + case MESSENGER_ERROR_TCP_SERVER: { + SET_ERROR_PARAMETER(error, TOX_ERR_NEW_PORT_ALLOC); + break; + } + default: { + SET_ERROR_PARAMETER(error, TOX_ERR_NEW_MALLOC); + break; + } } mono_time_free(tox->sys.mem, tox->mono_time);