Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cleanup: Fix a few more clang-tidy warnings. #2406

Merged
merged 1 commit into from
Sep 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -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
31 changes: 19 additions & 12 deletions other/analysis/run-clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ CHECKS="$CHECKS,-cppcoreguidelines-init-variables"
# 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
Expand All @@ -27,6 +29,10 @@ CHECKS="$CHECKS,-bugprone-reserved-identifier"
CHECKS="$CHECKS,-cert-dcl37-c"
CHECKS="$CHECKS,-cert-dcl51-cpp"

# Too restrictive. This makes sense if the branch clone is very large, but not
# if it's a single line. It can make the code less readable.
CHECKS="$CHECKS,-bugprone-branch-clone"

# We intentionally send some not null-terminated strings in tests and use it for
# the toxencryptsave magic number.
CHECKS="$CHECKS,-bugprone-not-null-terminated-result"
Expand All @@ -43,6 +49,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"
Expand All @@ -51,21 +65,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.
Expand All @@ -75,15 +81,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
Expand Down
2 changes: 1 addition & 1 deletion other/bootstrap_daemon/docker/tox-bootstrapd.sha256
Original file line number Diff line number Diff line change
@@ -1 +1 @@
3089925fd022dec8ec335435caf56f6ad5ccd2bf916f92f86a544da0ff654501 /usr/local/bin/tox-bootstrapd
619d28e6ecd0dbfbf8727753d44aa7eb1d8baa53cfcd4067f426141b4c132784 /usr/local/bin/tox-bootstrapd
4 changes: 0 additions & 4 deletions other/bootstrap_daemon/src/tox-bootstrapd.c
Original file line number Diff line number Diff line change
Expand Up @@ -178,11 +178,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;

Expand Down
4 changes: 1 addition & 3 deletions toxcore/DHT.c
Original file line number Diff line number Diff line change
Expand Up @@ -2466,9 +2466,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;
}
}
Expand Down
2 changes: 1 addition & 1 deletion toxcore/mono_time.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
5 changes: 2 additions & 3 deletions toxcore/net_crypto.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
17 changes: 11 additions & 6 deletions toxcore/tox.c
Original file line number Diff line number Diff line change
Expand Up @@ -812,12 +812,17 @@ 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;
}
case MESSENGER_ERROR_OTHER:
case MESSENGER_ERROR_NONE: {
SET_ERROR_PARAMETER(error, TOX_ERR_NEW_MALLOC);
break;
}
}

mono_time_free(tox->sys.mem, tox->mono_time);
Expand Down