From 30c939e4ab1e20070235c78bf6fefa99a5bc25b0 Mon Sep 17 00:00:00 2001 From: iphydf Date: Thu, 9 Dec 2021 21:13:49 +0000 Subject: [PATCH] cleanup: Fix some clang-tidy warnings and make them errors. The android warnings are disabled now because they suggest using linux-only extensions of libc. Useful for android indeed, but we're targeting non-android and non-linux systems as well. --- other/analysis/run-clang-tidy | 9 +-- other/bootstrap_daemon/src/log.c | 2 +- other/bootstrap_daemon/src/tox-bootstrapd.c | 10 ++- toxav/msi.c | 74 ++++++++++++--------- toxcore/DHT.c | 37 ++++++----- toxcore/tox.c | 7 +- toxcore/tox_api.c | 7 +- toxencryptsave/toxencryptsave.c | 8 ++- 8 files changed, 99 insertions(+), 55 deletions(-) diff --git a/other/analysis/run-clang-tidy b/other/analysis/run-clang-tidy index e4afb111..29cd4ab8 100755 --- a/other/analysis/run-clang-tidy +++ b/other/analysis/run-clang-tidy @@ -1,6 +1,8 @@ #!/bin/sh CHECKS="*" +CHECKS="$CHECKS,-android-cloexec-accept" +CHECKS="$CHECKS,-android-cloexec-fopen" CHECKS="$CHECKS,-bugprone-not-null-terminated-result" CHECKS="$CHECKS,-bugprone-reserved-identifier" CHECKS="$CHECKS,-bugprone-sizeof-expression" @@ -18,24 +20,24 @@ CHECKS="$CHECKS,-misc-unused-parameters" CHECKS="$CHECKS,-readability-else-after-return" CHECKS="$CHECKS,-readability-inconsistent-declaration-parameter-name" CHECKS="$CHECKS,-readability-magic-numbers" +CHECKS="$CHECKS,-readability-redundant-control-flow" ERRORS="*" +# TODO(iphydf): Maybe fix these? Otherwise don't show them, if they are useless. ERRORS="$ERRORS,-bugprone-branch-clone" ERRORS="$ERRORS,-bugprone-integer-division" ERRORS="$ERRORS,-bugprone-narrowing-conversions" ERRORS="$ERRORS,-clang-analyzer-core.NonNullParamChecker" ERRORS="$ERRORS,-clang-analyzer-core.NullDereference" ERRORS="$ERRORS,-clang-analyzer-optin.portability.UnixAPI" +ERRORS="$ERRORS,-clang-analyzer-unix.Malloc" ERRORS="$ERRORS,-clang-analyzer-valist.Uninitialized" ERRORS="$ERRORS,-cppcoreguidelines-avoid-non-const-global-variables" ERRORS="$ERRORS,-cppcoreguidelines-narrowing-conversions" ERRORS="$ERRORS,-google-readability-casting" ERRORS="$ERRORS,-misc-no-recursion" -ERRORS="$ERRORS,-readability-redundant-control-flow" # TODO(iphydf): Fix these. -ERRORS="$ERRORS,-android-cloexec-accept" -ERRORS="$ERRORS,-android-cloexec-fopen" ERRORS="$ERRORS,-bugprone-macro-parentheses" ERRORS="$ERRORS,-bugprone-posix-return" ERRORS="$ERRORS,-bugprone-signed-char-misuse" @@ -43,7 +45,6 @@ ERRORS="$ERRORS,-cert-err34-c" ERRORS="$ERRORS,-cert-str34-c" ERRORS="$ERRORS,-clang-analyzer-security.insecureAPI.strcpy" ERRORS="$ERRORS,-hicpp-uppercase-literal-suffix" -ERRORS="$ERRORS,-readability-non-const-parameter" ERRORS="$ERRORS,-readability-uppercase-literal-suffix" set -eux diff --git a/other/bootstrap_daemon/src/log.c b/other/bootstrap_daemon/src/log.c index 6f498b22..bc36b9c8 100644 --- a/other/bootstrap_daemon/src/log.c +++ b/other/bootstrap_daemon/src/log.c @@ -11,7 +11,7 @@ #include "log_backend_stdout.h" #include "log_backend_syslog.h" -#define INVALID_BACKEND (LOG_BACKEND)-1u +#define INVALID_BACKEND ((LOG_BACKEND)-1u) static LOG_BACKEND current_backend = INVALID_BACKEND; bool log_open(LOG_BACKEND backend) diff --git a/other/bootstrap_daemon/src/tox-bootstrapd.c b/other/bootstrap_daemon/src/tox-bootstrapd.c index 5afe2776..583826c3 100644 --- a/other/bootstrap_daemon/src/tox-bootstrapd.c +++ b/other/bootstrap_daemon/src/tox-bootstrapd.c @@ -42,7 +42,13 @@ #include "log.h" -#define SLEEP_MILLISECONDS(MS) usleep(1000*MS) +static void sleep_milliseconds(uint32_t ms) +{ + struct timespec req; + req.tv_sec = ms / 1000; + req.tv_nsec = (long)ms % 1000 * 1000 * 1000; + nanosleep(&req, nullptr); +} // Uses the already existing key or creates one if it didn't exist // @@ -514,7 +520,7 @@ int main(int argc, char *argv[]) waiting_for_dht_connection = 0; } - SLEEP_MILLISECONDS(30); + sleep_milliseconds(30); } switch (caught_signal) { diff --git a/toxav/msi.c b/toxav/msi.c index efa21bbf..f753baf7 100644 --- a/toxav/msi.c +++ b/toxav/msi.c @@ -315,32 +315,40 @@ static void msg_init(MSIMessage *dest, MSIRequest request) dest->request.exists = true; dest->request.value = request; } + +static bool check_size(const Logger *log, const uint8_t *bytes, int *constraint, uint8_t size) +{ + *constraint -= 2 + size; + + if (*constraint < 1) { + LOGGER_ERROR(log, "Read over length!"); + return false; + } + + if (bytes[1] != size) { + LOGGER_ERROR(log, "Invalid data size!"); + return false; + } + + return true; +} + +/* Assumes size == 1 */ +static bool check_enum_high(const Logger *log, const uint8_t *bytes, uint8_t enum_high) +{ + if (bytes[2] > enum_high) { + LOGGER_ERROR(log, "Failed enum high limit!"); + return false; + } + + return true; +} + + static int msg_parse_in(const Logger *log, MSIMessage *dest, const uint8_t *data, uint16_t length) { /* Parse raw data received from socket into MSIMessage struct */ -#define CHECK_SIZE(bytes, constraint, size) \ - do { \ - constraint -= 2 + size; \ - if (constraint < 1) { \ - LOGGER_ERROR(log, "Read over length!"); \ - return -1; \ - } \ - if (bytes[1] != size) { \ - LOGGER_ERROR(log, "Invalid data size!"); \ - return -1; \ - } \ - } while (0) - - /* Assumes size == 1 */ -#define CHECK_ENUM_HIGH(bytes, enum_high) \ - do { \ - if (bytes[2] > enum_high) { \ - LOGGER_ERROR(log, "Failed enum high limit!"); \ - return -1; \ - } \ - } while (0) - assert(dest); if (length == 0 || data[length - 1]) { /* End byte must have value 0 */ @@ -356,8 +364,11 @@ static int msg_parse_in(const Logger *log, MSIMessage *dest, const uint8_t *data while (*it) {/* until end byte is hit */ switch (*it) { case ID_REQUEST: { - CHECK_SIZE(it, size_constraint, 1); - CHECK_ENUM_HIGH(it, REQU_POP); + if (!check_size(log, it, &size_constraint, 1) || + !check_enum_high(log, it, REQU_POP)) { + return -1; + } + dest->request.value = (MSIRequest)it[2]; dest->request.exists = true; it += 3; @@ -365,8 +376,11 @@ static int msg_parse_in(const Logger *log, MSIMessage *dest, const uint8_t *data } case ID_ERROR: { - CHECK_SIZE(it, size_constraint, 1); - CHECK_ENUM_HIGH(it, MSI_E_UNDISCLOSED); + if (!check_size(log, it, &size_constraint, 1) || + !check_enum_high(log, it, MSI_E_UNDISCLOSED)) { + return -1; + } + dest->error.value = (MSIError)it[2]; dest->error.exists = true; it += 3; @@ -374,7 +388,10 @@ static int msg_parse_in(const Logger *log, MSIMessage *dest, const uint8_t *data } case ID_CAPABILITIES: { - CHECK_SIZE(it, size_constraint, 1); + if (!check_size(log, it, &size_constraint, 1)) { + return -1; + } + dest->capabilities.value = it[2]; dest->capabilities.exists = true; it += 3; @@ -393,9 +410,6 @@ static int msg_parse_in(const Logger *log, MSIMessage *dest, const uint8_t *data return -1; } -#undef CHECK_ENUM_HIGH -#undef CHECK_SIZE - return 0; } static uint8_t *msg_parse_header_out(MSIHeaderID id, uint8_t *dest, const void *value, uint8_t value_len, diff --git a/toxcore/DHT.c b/toxcore/DHT.c index 07409dc0..d4a6c6dc 100644 --- a/toxcore/DHT.c +++ b/toxcore/DHT.c @@ -620,30 +620,37 @@ int unpack_nodes(Node_format *nodes, uint16_t max_num_nodes, uint16_t *processed * * return index or UINT32_MAX if not found. */ -#define INDEX_OF_PK(array, size, pk) \ - do { \ - for (uint32_t i = 0; i < size; ++i) { \ - if (id_equal(array[i].public_key, pk)) { \ - return i; \ - } \ - } \ - \ - return UINT32_MAX; \ - } while (0) - static uint32_t index_of_client_pk(const Client_data *array, uint32_t size, const uint8_t *pk) { - INDEX_OF_PK(array, size, pk); + for (uint32_t i = 0; i < size; ++i) { + if (id_equal(array[i].public_key, pk)) { + return i; + } + } + + return UINT32_MAX; } static uint32_t index_of_friend_pk(const DHT_Friend *array, uint32_t size, const uint8_t *pk) { - INDEX_OF_PK(array, size, pk); + for (uint32_t i = 0; i < size; ++i) { + if (id_equal(array[i].public_key, pk)) { + return i; + } + } + + return UINT32_MAX; } static uint32_t index_of_node_pk(const Node_format *array, uint32_t size, const uint8_t *pk) { - INDEX_OF_PK(array, size, pk); + for (uint32_t i = 0; i < size; ++i) { + if (id_equal(array[i].public_key, pk)) { + return i; + } + } + + return UINT32_MAX; } /* Find index of Client_data with ip_port equal to param ip_port. @@ -2181,7 +2188,7 @@ static uint16_t nat_getports(uint16_t *portlist, IP_Port *ip_portlist, uint16_t return num; } -static void punch_holes(DHT *dht, IP ip, uint16_t *port_list, uint16_t numports, uint16_t friend_num) +static void punch_holes(DHT *dht, IP ip, const uint16_t *port_list, uint16_t numports, uint16_t friend_num) { if (!dht->hole_punching_enabled) { return; diff --git a/toxcore/tox.c b/toxcore/tox.c index cc2bb9f0..e6acebe1 100644 --- a/toxcore/tox.c +++ b/toxcore/tox.c @@ -30,7 +30,12 @@ #include "../toxencryptsave/defines.h" -#define SET_ERROR_PARAMETER(param, x) do { if (param) { *param = x; } } while (0) +#define SET_ERROR_PARAMETER(param, x) \ + do { \ + if (param) { \ + *param = x; \ + } \ + } while (0) //!TOKSTYLE- static_assert(TOX_HASH_LENGTH == CRYPTO_SHA256_SIZE, diff --git a/toxcore/tox_api.c b/toxcore/tox_api.c index 63b4bea3..9bf554b6 100644 --- a/toxcore/tox_api.c +++ b/toxcore/tox_api.c @@ -5,7 +5,12 @@ #include #include -#define SET_ERROR_PARAMETER(param, x) do { if (param) { *param = x; } } while (0) +#define SET_ERROR_PARAMETER(param, x) \ + do { \ + if (param) { \ + *param = x; \ + } \ + } while (0) //!TOKSTYLE- diff --git a/toxencryptsave/toxencryptsave.c b/toxencryptsave/toxencryptsave.c index be4bcaf2..5889aab9 100644 --- a/toxencryptsave/toxencryptsave.c +++ b/toxencryptsave/toxencryptsave.c @@ -14,7 +14,6 @@ #include "../toxcore/crypto_core.h" #include "defines.h" #include "toxencryptsave.h" -#define SET_ERROR_PARAMETER(param, x) do { if (param) { *param = x; } } while (0) #ifdef VANILLA_NACL #include @@ -38,6 +37,13 @@ static_assert(TOX_PASS_ENCRYPTION_EXTRA_LENGTH == (crypto_box_MACBYTES + crypto_ "TOX_PASS_ENCRYPTION_EXTRA_LENGTH is assumed to be equal to (crypto_box_MACBYTES + crypto_box_NONCEBYTES + crypto_pwhash_scryptsalsa208sha256_SALTBYTES + TOX_ENC_SAVE_MAGIC_LENGTH)"); //!TOKSTYLE+ +#define SET_ERROR_PARAMETER(param, x) \ + do { \ + if (param) { \ + *param = x; \ + } \ + } while (0) + uint32_t tox_pass_salt_length(void) { return TOX_PASS_SALT_LENGTH;