From: Oto Šťáva Date: Mon, 29 Apr 2024 13:09:01 +0000 (+0200) Subject: Silence Clang-Tidy X-Git-Tag: v5.7.3~5^2~3 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c1bdcd06cb20948971c0110e6f28a4254828ea23;p=thirdparty%2Fknot-resolver.git Silence Clang-Tidy This commit makes lots of changes to the C code to appease the Clang-Tidy linter. Some of the less obvious ones are due to C's weird semantics regarding handling of numeric literals. We also disable a bunch of the detections because they are super-pedantic, arguably useless, or we have our own unwritten coding style rules that solve the issues. --- diff --git a/.clang-tidy b/.clang-tidy index b496044c4..ecc9a6214 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,6 +1,42 @@ --- -Checks: 'bugprone-*,cert-*,-cert-dcl03-c,-clang-analyzer-unix.Malloc,-clang-analyzer-deadcode.DeadStores,-clang-analyzer-valist.Uninitialized,readability-*,-readability-braces-*,-readability-else-after-return,-readability-redundant-declaration,-readability-non-const-parameter,google-readability-casting,misc-*,-misc-static-assert,-misc-macro-parentheses,-misc-unused-parameters' -WarningsAsErrors: 'cert-*,misc-*,readability-*,clang-analyzer-*,-readability-non-const-parameter' +Checks: + - bugprone-* + - cert-* + - google-readability-casting + - misc-* + - readability-* + + - -bugprone-assignment-in-if-condition # we explicitly put assignments into parentheses so they are very visible + - -bugprone-branch-clone + - -bugprone-easily-swappable-parameters + - -bugprone-inc-dec-in-conditions + - -bugprone-multi-level-implicit-pointer-conversion + - -bugprone-narrowing-conversions + - -bugprone-sizeof-expression # may be useful, but it's utterly broken + - -bugprone-suspicious-string-compare + - -cert-dcl03-c + - -clang-analyzer-deadcode.DeadStores + - -clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling + - -clang-analyzer-unix.Malloc + - -clang-analyzer-valist.Uninitialized + - -clang-analyzer-optin.core.EnumCastOutOfRange # libknot uses enums as flags + - -misc-include-cleaner + - -misc-macro-parentheses + - -misc-no-recursion + - -misc-static-assert + - -misc-unused-parameters + - -readability-avoid-nested-conditional-operator + - -readability-avoid-unconditional-preprocessor-if + - -readability-braces-* + - -readability-cognitive-complexity + - -readability-else-after-return + - -readability-function-cognitive-complexity + - -readability-identifier-length + - -readability-isolate-declaration + - -readability-magic-numbers + - -readability-non-const-parameter + - -readability-redundant-declaration +WarningsAsErrors: 'cert-*,clang-analyzer-*,misc-*,readability-*,-readability-non-const-parameter' HeaderFilterRegex: 'contrib/ucw/*.h' CheckOptions: - key: readability-identifier-naming diff --git a/daemon/bindings/net.c b/daemon/bindings/net.c index f1fa6f3a3..0075d0f2d 100644 --- a/daemon/bindings/net.c +++ b/daemon/bindings/net.c @@ -470,7 +470,7 @@ static int net_interfaces(lua_State *L) /* Hardware address. */ char *p = buf; for (int k = 0; k < sizeof(iface.phys_addr); ++k) { - sprintf(p, "%.2x:", (uint8_t)iface.phys_addr[k]); + (void)sprintf(p, "%.2x:", (uint8_t)iface.phys_addr[k]); p += 3; } p[-1] = '\0'; @@ -794,7 +794,7 @@ static int net_tls_client(lua_State *L) /* Sort the strings for easier comparison later. */ if (newcfg->ca_files.len) { qsort(&newcfg->ca_files.at[0], newcfg->ca_files.len, - sizeof(newcfg->ca_files.at[0]), strcmp_p); + array_member_size(newcfg->ca_files), strcmp_p); } } lua_pop(L, 1); @@ -834,7 +834,7 @@ static int net_tls_client(lua_State *L) /* Sort the raw strings for easier comparison later. */ if (newcfg->pins.len) { qsort(&newcfg->pins.at[0], newcfg->pins.len, - sizeof(newcfg->pins.at[0]), cmp_sha256); + array_member_size(newcfg->pins), cmp_sha256); } } lua_pop(L, 1); @@ -1042,7 +1042,11 @@ static int net_tls_sticket_secret_file(lua_State *L) STR(net_tls_sticket_MIN_SECRET_LEN) " bytes", file_name); } - fclose(fp); + if (fclose(fp) == EOF) { + lua_error_p(L, + "net.tls_sticket_secret_file - reading of file '%s' failed", + file_name); + } struct network *net = &the_worker->engine->net; diff --git a/daemon/engine.c b/daemon/engine.c index 1f9f10051..8c00a5bef 100644 --- a/daemon/engine.c +++ b/daemon/engine.c @@ -52,9 +52,6 @@ #define TCP_BACKLOG_DEFAULT 128 #endif -/* Cleanup engine state every 5 minutes */ -const size_t CLEANUP_TIMER = 5*60*1000; - /* Execute byte code */ #define l_dobytecode(L, arr, len, name) \ (luaL_loadbuffer((L), (arr), (len), (name)) || lua_pcall((L), 0, LUA_MULTRET, 0)) @@ -609,7 +606,7 @@ int init_lua(struct engine *engine) { #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wformat" /* %1$ is not in C standard */ /* Save original package.path to package._path */ - snprintf(l_paths, MAXPATHLEN - 1, + (void)snprintf(l_paths, MAXPATHLEN - 1, "if package._path == nil then package._path = package.path end\n" "package.path = '%1$s/?.lua;%1$s/?/init.lua;'..package._path\n" "if package._cpath == nil then package._cpath = package.cpath end\n" diff --git a/daemon/io.c b/daemon/io.c index 6d548d777..9299ff2ad 100644 --- a/daemon/io.c +++ b/daemon/io.c @@ -151,7 +151,7 @@ static int family_to_freebind_option(sa_family_t sa_family, int *level, int *nam #define LOG_NO_FB kr_log_error(NETWORK, "your system does not support 'freebind', " \ "please remove it from your configuration\n") switch (sa_family) { - case AF_INET: + case AF_INET: // NOLINT(bugprone-branch-clone): The branches are only cloned for specific macro configs *level = IPPROTO_IP; #if defined(IP_FREEBIND) *name = IP_FREEBIND; @@ -510,7 +510,7 @@ static ssize_t tls_send(const uint8_t *buf, const size_t len, struct session *se } #endif -static void _tcp_accept(uv_stream_t *master, int status, bool tls, bool http) +static void tcp_accept_internal(uv_stream_t *master, int status, bool tls, bool http) { if (status != 0) { return; @@ -631,18 +631,18 @@ static void _tcp_accept(uv_stream_t *master, int status, bool tls, bool http) static void tcp_accept(uv_stream_t *master, int status) { - _tcp_accept(master, status, false, false); + tcp_accept_internal(master, status, false, false); } static void tls_accept(uv_stream_t *master, int status) { - _tcp_accept(master, status, true, false); + tcp_accept_internal(master, status, true, false); } #if ENABLE_DOH2 static void https_accept(uv_stream_t *master, int status) { - _tcp_accept(master, status, true, true); + tcp_accept_internal(master, status, true, true); } #endif @@ -834,16 +834,25 @@ void io_tty_process_input(uv_stream_t *stream, ssize_t nread, const uv_buf_t *bu len_s = 0; } uint32_t len_n = htonl(len_s); - fwrite(&len_n, sizeof(len_n), 1, out); - if (len_s > 0) - fwrite(message, len_s, 1, out); + if (fwrite(&len_n, sizeof(len_n), 1, out) != 1) + goto finish; + if (len_s > 0) { + if (fwrite(message, len_s, 1, out) != 1) + goto finish; + } } else { - if (message) - fprintf(out, "%s", message); - if (message || !args->quiet) - fprintf(out, "\n"); - if (!args->quiet) - fprintf(out, "> "); + if (message) { + if (fprintf(out, "%s", message) < 0) + goto finish; + } + if (message || !args->quiet) { + if (fprintf(out, "\n") < 0) + goto finish; + } + if (!args->quiet) { + if (fprintf(out, "> ") < 0) + goto finish; + } } /* Duplicate command and output to logs */ @@ -865,7 +874,7 @@ void io_tty_process_input(uv_stream_t *stream, ssize_t nread, const uv_buf_t *bu finish: /* Close if redirected */ if (stream_fd != STDIN_FILENO) { - fclose(out); + (void)fclose(out); } } diff --git a/daemon/main.c b/daemon/main.c index 41a55ad52..a346a5c83 100644 --- a/daemon/main.c +++ b/daemon/main.c @@ -425,9 +425,9 @@ int main(int argc, char **argv) { kr_log_group_reset(); if (setvbuf(stdout, NULL, _IONBF, 0) || setvbuf(stderr, NULL, _IONBF, 0)) { - kr_log_error(SYSTEM, "failed to to set output buffering (ignored): %s\n", + kr_log_error(SYSTEM, "failed to set output buffering (ignored): %s\n", strerror(errno)); - fflush(stderr); + (void)fflush(stderr); } if (strcmp("linux", OPERATING_SYSTEM) != 0) kr_log_warning(SYSTEM, "Knot Resolver is tested on Linux, other platforms might exhibit bugs.\n" @@ -490,7 +490,7 @@ int main(int argc, char **argv) if (ret) { kr_log_error(SYSTEM, "failed to get or set file-descriptor limit: %s\n", strerror(errno)); - } else if (rlim.rlim_cur < 512*1024) { + } else if (rlim.rlim_cur < (rlim_t)512 * 1024) { kr_log_warning(SYSTEM, "warning: hard limit for number of file-descriptors is only %ld but recommended value is 524288\n", (long)rlim.rlim_cur); } diff --git a/daemon/proxyv2.c b/daemon/proxyv2.c index aedbb91a7..819700320 100644 --- a/daemon/proxyv2.c +++ b/daemon/proxyv2.c @@ -279,6 +279,7 @@ ssize_t proxy_process_header(struct proxy_result *out, struct session *s, &addr->ipv6_addr.dst_addr, sizeof(out->dst_addr.ip6.sin6_addr.s6_addr)); break; + default:; /* Keep zero from initializer. */ } /* Process additional information */ @@ -287,7 +288,7 @@ ssize_t proxy_process_header(struct proxy_result *out, struct session *s, case TLV_TYPE_SSL: out->has_tls = true; break; - /* TODO: add more TLV types if needed */ + default:; /* Ignore others - add more if needed */ } } diff --git a/daemon/session.c b/daemon/session.c index ed0ff6869..91d3c39e1 100644 --- a/daemon/session.c +++ b/daemon/session.c @@ -13,7 +13,7 @@ #include "daemon/proxyv2.h" #include "lib/generic/queue.h" -#define TLS_CHUNK_SIZE (16 * 1024) +#define TLS_CHUNK_SIZE ((size_t)16 * 1024) /* Initial max frame size: https://tools.ietf.org/html/rfc7540#section-6.5.2 */ #define HTTP_MAX_FRAME_SIZE 16384 diff --git a/daemon/tls.c b/daemon/tls.c index 2e1631b97..0ab396827 100644 --- a/daemon/tls.c +++ b/daemon/tls.c @@ -23,7 +23,7 @@ #include "daemon/worker.h" #include "daemon/session.h" -#define EPHEMERAL_CERT_EXPIRATION_SECONDS_RENEW_BEFORE (60*60*24*7) +#define EPHEMERAL_CERT_EXPIRATION_SECONDS_RENEW_BEFORE ((time_t)60*60*24*7) #define GNUTLS_PIN_MIN_VERSION 0x030400 #define VERBOSE_MSG(cl_side, ...)\ @@ -659,7 +659,7 @@ static int str_replace(char **where_ptr, const char *with) return kr_ok(); } -static time_t _get_end_entity_expiration(gnutls_certificate_credentials_t creds) +static time_t get_end_entity_expiration(gnutls_certificate_credentials_t creds) { gnutls_datum_t data; gnutls_x509_crt_t cert = NULL; @@ -731,7 +731,7 @@ int tls_certificate_set(struct network *net, const char *tls_cert, const char *t return kr_error(EINVAL); } /* record the expiration date: */ - tls_credentials->valid_until = _get_end_entity_expiration(tls_credentials->credentials); + tls_credentials->valid_until = get_end_entity_expiration(tls_credentials->credentials); /* Exchange the x509 credentials */ struct tls_credentials *old_credentials = net->tls_credentials; diff --git a/daemon/tls.h b/daemon/tls.h index af1f5c98e..c30444bea 100644 --- a/daemon/tls.h +++ b/daemon/tls.h @@ -30,7 +30,7 @@ * So it takes 2 RTT. * As we use session tickets, there are additional messages, add one RTT mode. */ - #define TLS_MAX_HANDSHAKE_TIME (KR_CONN_RTT_MAX * 3) + #define TLS_MAX_HANDSHAKE_TIME (KR_CONN_RTT_MAX * (uint64_t)3) /** Transport session (opaque). */ struct session; diff --git a/daemon/tls_ephemeral_credentials.c b/daemon/tls_ephemeral_credentials.c index ff4682f98..0d9ec6db6 100644 --- a/daemon/tls_ephemeral_credentials.c +++ b/daemon/tls_ephemeral_credentials.c @@ -17,19 +17,19 @@ #define EPHEMERAL_PRIVKEY_FILENAME "ephemeral_key.pem" #define INVALID_HOSTNAME "dns-over-tls.invalid" -#define EPHEMERAL_CERT_EXPIRATION_SECONDS (60*60*24*90) +#define EPHEMERAL_CERT_EXPIRATION_SECONDS ((time_t)60*60*24*90) /* This is an attempt to grab an exclusive, advisory, non-blocking * lock based on a filename. At the moment it's POSIX-only, but it * should be abstract enough of an interface to make an implementation * for non-posix systems if anyone cares. */ typedef int lock_t; -static bool _lock_is_invalid(lock_t lock) +static bool lock_is_invalid(lock_t lock) { return lock == -1; } /* a blocking lock on a given filename */ -static lock_t _lock_filename(const char *fname) +static lock_t lock_filename(const char *fname) { lock_t lockfd = open(fname, O_RDONLY|O_CREAT, 0400); if (lockfd == -1) @@ -41,9 +41,9 @@ static lock_t _lock_filename(const char *fname) } return lockfd; /* for cleanup later */ } -static void _lock_unlock(lock_t *lock, const char *fname) +static void lock_unlock(lock_t *lock, const char *fname) { - if (lock && !_lock_is_invalid(*lock)) { + if (lock && !lock_is_invalid(*lock)) { flock(*lock, LOCK_UN); close(*lock); *lock = -1; @@ -61,8 +61,8 @@ static gnutls_x509_privkey_t get_ephemeral_privkey (void) /* Take a lock to ensure that two daemons started concurrently * with a shared cache don't both create the same privkey: */ - lock = _lock_filename(EPHEMERAL_PRIVKEY_FILENAME ".lock"); - if (_lock_is_invalid(lock)) { + lock = lock_filename(EPHEMERAL_PRIVKEY_FILENAME ".lock"); + if (lock_is_invalid(lock)) { kr_log_error(TLS, "unable to lock lockfile " EPHEMERAL_PRIVKEY_FILENAME ".lock\n"); goto done; } @@ -141,7 +141,7 @@ static gnutls_x509_privkey_t get_ephemeral_privkey (void) } } done: - _lock_unlock(&lock, EPHEMERAL_PRIVKEY_FILENAME ".lock"); + lock_unlock(&lock, EPHEMERAL_PRIVKEY_FILENAME ".lock"); if (datafd != -1) { close(datafd); } @@ -219,7 +219,7 @@ struct tls_credentials * tls_get_ephemeral_credentials(struct engine *engine) if ((privkey = get_ephemeral_privkey()) == NULL) { goto failure; } - if ((cert = get_ephemeral_cert(privkey, creds->ephemeral_servicename, now - 60*15, creds->valid_until)) == NULL) { + if ((cert = get_ephemeral_cert(privkey, creds->ephemeral_servicename, now - ((time_t)60 * 15), creds->valid_until)) == NULL) { goto failure; } if ((err = gnutls_certificate_set_x509_key(creds->credentials, &cert, 1, privkey)) < 0) { diff --git a/daemon/tls_session_ticket-srv.c b/daemon/tls_session_ticket-srv.c index b19890300..26d41862f 100644 --- a/daemon/tls_session_ticket-srv.c +++ b/daemon/tls_session_ticket-srv.c @@ -188,7 +188,7 @@ static void tst_key_check(uv_timer_t *timer, bool force_update) const uint64_t remain_ms = (tv_sec_next - now.tv_sec - 1) * (uint64_t)1000 + ms_until_second + 1; /* ^ +1 because we don't want to wake up half a millisecond before the epoch! */ - if (kr_fails_assert(remain_ms < (TST_KEY_LIFETIME + 1 /*rounding tolerance*/) * 1000)) + if (kr_fails_assert(remain_ms < ((uint64_t)TST_KEY_LIFETIME + 1 /*rounding tolerance*/) * 1000)) return; kr_log_debug(TLS, "session ticket: epoch %"PRIu64 ", scheduling rotation check in %"PRIu64" ms\n", diff --git a/daemon/udp_queue.c b/daemon/udp_queue.c index 1f8ff39ec..7ed600a9e 100644 --- a/daemon/udp_queue.c +++ b/daemon/udp_queue.c @@ -110,11 +110,11 @@ void udp_queue_push(int fd, struct kr_request *req, struct qr_task *task) /* Get a valid correct queue. */ if (fd >= state.udp_queues_len) { const int new_len = fd + 1; - state.udp_queues = realloc(state.udp_queues, - sizeof(state.udp_queues[0]) * new_len); + state.udp_queues = realloc(state.udp_queues, // NOLINT(bugprone-suspicious-realloc-usage): we just abort() below, so it's fine + sizeof(state.udp_queues[0]) * new_len); // NOLINT(bugprone-sizeof-expression): false-positive if (!state.udp_queues) abort(); memset(state.udp_queues + state.udp_queues_len, 0, - sizeof(state.udp_queues[0]) * (new_len - state.udp_queues_len)); + sizeof(state.udp_queues[0]) * (new_len - state.udp_queues_len)); // NOLINT(bugprone-sizeof-expression): false-positive state.udp_queues_len = new_len; } if (unlikely(state.udp_queues[fd] == NULL)) diff --git a/daemon/worker.c b/daemon/worker.c index 8b6b49e67..12c08f160 100644 --- a/daemon/worker.c +++ b/daemon/worker.c @@ -195,7 +195,7 @@ static inline struct mempool *pool_borrow(struct worker_ctx *worker) { /* The implementation used to have extra caching layer, * but it didn't work well. Now it's very simple. */ - return mp_new(16 * 1024); + return mp_new((size_t)16 * 1024); } /** Return a mempool. */ static inline void pool_release(struct worker_ctx *worker, struct mempool *mp) diff --git a/daemon/zimport.c b/daemon/zimport.c index af21a1594..39799b604 100644 --- a/daemon/zimport.c +++ b/daemon/zimport.c @@ -98,7 +98,7 @@ static int key_get(char buf[KEY_LEN], const knot_dname_t *name, char *lf = (char *)knot_dname_lf(name, (uint8_t *)buf); if (kr_fails_assert(lf && key_p)) return kr_error(EINVAL); - int len = lf[0]; + int len = (unsigned char)lf[0]; lf++; // point to start of data *key_p = lf; // Check that LF is right-aligned to KNOT_DNAME_MAXLEN in buf. @@ -282,7 +282,7 @@ do_digest: // hexdump the hash for logging char hash_str[digs[i].size * 2 + 1]; for (ssize_t j = 0; j < digs[i].size; ++j) - sprintf(hash_str + 2*j, "%02x", digs[i].data[j]); + (void)sprintf(hash_str + 2*j, "%02x", digs[i].data[j]); if (!z_import->digests[i].expected) { kr_log_error(PREFILL, "no ZONEMD found; computed hash: %s\n", @@ -560,7 +560,7 @@ int zi_zone_import(const zi_config_t config) if (kr_fails_assert(c && c->zone_file)) return kr_error(EINVAL); - knot_mm_t *pool = mm_ctx_mempool2(1024 * 1024); + knot_mm_t *pool = mm_ctx_mempool2((size_t)1024 * 1024); zone_import_ctx_t *z_import = mm_calloc(pool, 1, sizeof(*z_import)); if (!z_import) return kr_error(ENOMEM); z_import->pool = pool; diff --git a/lib/cache/peek.c b/lib/cache/peek.c index 191e85567..f0bb79cce 100644 --- a/lib/cache/peek.c +++ b/lib/cache/peek.c @@ -174,6 +174,7 @@ int peek_nosync(kr_layer_t *ctx, knot_pkt_t *pkt) knot_db_val_bound(v), new_ttl); return ret == kr_ok() ? KR_STATE_DONE : ctx->state; } + default:; // Continue below } /* We have to try proving from NSEC*. */ diff --git a/lib/dnssec.c b/lib/dnssec.c index 12b8f2021..eb4b33b1a 100644 --- a/lib/dnssec.c +++ b/lib/dnssec.c @@ -362,7 +362,7 @@ static int kr_rrset_validate_with_key(kr_rrset_validation_ctx_t *vctx, const int covered_labels = knot_dname_labels(covered->owner, NULL) - knot_dname_is_wildcard(covered->owner); - for (uint16_t i = 0; i < vctx->rrs->len; ++i) { + for (size_t i = 0; i < vctx->rrs->len; ++i) { /* Consider every RRSIG that matches and comes from the same query. */ const knot_rrset_t *rrsig = vctx->rrs->at[i]->rr; const bool ok = vctx->rrs->at[i]->qry_uid == vctx->qry_uid diff --git a/lib/generic/array.h b/lib/generic/array.h index 9f351189d..9bea546be 100644 --- a/lib/generic/array.h +++ b/lib/generic/array.h @@ -113,7 +113,7 @@ static inline void array_std_free(void *baton, void *p) * Mempool usage: pass kr_memreserve and a knot_mm_t* . * @return 0 if success, <0 on failure */ #define array_reserve_mm(array, n, reserve, baton) \ - (reserve)((baton), (void **) &(array).at, sizeof((array).at[0]), (n), &(array).cap) + (reserve)((baton), (void **) &(array).at, array_member_size((array)), (n), &(array).cap) /** * Push value at the end of the array, resize it if necessary. @@ -122,9 +122,9 @@ static inline void array_std_free(void *baton, void *p) * @return element index on success, <0 on failure */ #define array_push_mm(array, val, reserve, baton) \ - (int)((array).len < (array).cap ? ((array).at[(array).len] = val, (array).len++) \ + (int)((array).len < (array).cap ? ((array).at[(array).len] = (val), (array).len++) \ : (array_reserve_mm(array, ((array).cap + 1), reserve, baton) < 0 ? -1 \ - : ((array).at[(array).len] = val, (array).len++))) + : ((array).at[(array).len] = (val), (array).len++))) /** * Push value at the end of the array, resize it if necessary (plain malloc/free). @@ -152,6 +152,12 @@ static inline void array_std_free(void *baton, void *p) * @warning Undefined if the array is empty. */ #define array_tail(array) \ - (array).at[(array).len - 1] + (array).at[(array).len - 1] + +/** + * Return the size of a singular member in the array. + */ +#define array_member_size(array) \ + (sizeof((array).at[0])) // NOLINT(bugprone-sizeof-expression): usually a false-positive /** @} */ diff --git a/lib/generic/lru.h b/lib/generic/lru.h index 448c1b92f..1c1dd81ac 100644 --- a/lib/generic/lru.h +++ b/lib/generic/lru.h @@ -130,7 +130,10 @@ #define lru_get_new(table, key_, len_, is_new) \ (__typeof__((table)->pdata_t)) \ lru_get_impl(&(table)->lru, (key_), (len_), \ - sizeof(*(table)->pdata_t), true, is_new) + lru_member_size((table)), true, is_new) + +#define lru_member_size(table) \ + (sizeof(*(table)->pdata_t)) // NOLINT(bugprone-sizeof-expression): usually a false-positive /** * @brief Apply a function to every item in LRU. diff --git a/lib/generic/queue.c b/lib/generic/queue.c index 5bed153ef..29609dd2e 100644 --- a/lib/generic/queue.c +++ b/lib/generic/queue.c @@ -62,7 +62,7 @@ void * queue_push_impl(struct queue *q) if (t->begin * 2 >= t->cap) { /* Utilization is below 50%, so let's shift (no overlap). * (size_t cast is to avoid unintended sign-extension) */ - memcpy(t->data, t->data + t->begin * q->item_size, + memcpy(t->data, t->data + t->begin * (size_t)q->item_size, (size_t) (t->end - t->begin) * (size_t) q->item_size); t->end -= t->begin; t->begin = 0; @@ -76,7 +76,7 @@ void * queue_push_impl(struct queue *q) kr_require(t->end < t->cap); ++(q->len); ++(t->end); - return t->data + q->item_size * (t->end - 1); + return t->data + (size_t)q->item_size * (t->end - 1); } /* Return pointer to the space for the new element. */ @@ -98,8 +98,8 @@ void * queue_push_head_impl(struct queue *q) * Computations here are simplified due to h->begin == 0. * (size_t cast is to avoid unintended sign-extension) */ const int cnt = h->end; - memcpy(h->data + (h->cap - cnt) * q->item_size, h->data, - (size_t) cnt * (size_t) q->item_size); + memcpy(h->data + ((size_t)h->cap - cnt) * q->item_size, h->data, + (size_t)cnt * (size_t)q->item_size); h->begin = h->cap - cnt; h->end = h->cap; } else { @@ -113,7 +113,7 @@ void * queue_push_head_impl(struct queue *q) kr_require(h->begin > 0); --(h->begin); ++(q->len); - return h->data + q->item_size * h->begin; + return h->data + (size_t)q->item_size * h->begin; } void queue_pop_impl(struct queue *q) diff --git a/lib/generic/queue.h b/lib/generic/queue.h index 3fa52cea6..fc2a86f3c 100644 --- a/lib/generic/queue.h +++ b/lib/generic/queue.h @@ -71,7 +71,7 @@ /** @brief Initialize a queue. You can malloc() it the usual way. */ #define queue_init(q) do { \ (void)(((__typeof__(((q).pdata_t)))0) == (void *)0); /* typecheck queue_t */ \ - queue_init_impl(&(q).queue, sizeof(*(q).pdata_t)); \ + queue_init_impl(&(q).queue, queue_member_size((q))); \ } while (false) /** @brief De-initialize a queue: make it invalid and free any inner allocations. */ @@ -105,6 +105,10 @@ #define queue_len(q) \ ((const size_t)(q).queue.len) +/** @brief Return the size of a single element in the queue. */ +#define queue_member_size(q) \ + (sizeof(*(q).pdata_t)) // NOLINT(bugprone-sizeof-expression): usually a false-positive + /** @brief Type for queue iterator, parametrized by value type. * It's a simple structure that owns no other resources. diff --git a/lib/generic/trie.c b/lib/generic/trie.c index f9aceda79..21254eb4a 100644 --- a/lib/generic/trie.c +++ b/lib/generic/trie.c @@ -470,6 +470,10 @@ static int ns_longer_alloc(nstack_t *ns) memcpy(st, ns->stack, ns->len * sizeof(node_t *)); } else { st = realloc(ns->stack, new_size); + if (st == NULL) { + free(ns->stack); // left behind by realloc, callers bail out + ns->stack = NULL; + } } if (st == NULL) return KNOT_ENOMEM; diff --git a/lib/layer/validate.c b/lib/layer/validate.c index 3bdb205c4..af20b2e45 100644 --- a/lib/layer/validate.c +++ b/lib/layer/validate.c @@ -709,7 +709,7 @@ static int check_validation_result(kr_layer_t *ctx, const knot_pkt_t *pkt, ranke invalid_entry = entry; break; } else if (kr_rank_test(entry->rank, KR_RANK_MISSING) && - !invalid_entry) { + !invalid_entry) { // NOLINT(bugprone-branch-clone) invalid_entry = entry; } else if (kr_rank_test(entry->rank, KR_RANK_OMIT)) { continue; diff --git a/lib/log.c b/lib/log.c index 9c8c7a6f3..9fb16e910 100644 --- a/lib/log.c +++ b/lib/log.c @@ -124,7 +124,7 @@ void kr_log_fmt(enum kr_log_group group, kr_log_level_t level, const char *file, } va_start(args, fmt); - vfprintf(stream, fmt, args); + (void)vfprintf(stream, fmt, args); va_end(args); } } diff --git a/lib/resolve.c b/lib/resolve.c index 710b1ff2b..d8198c34b 100644 --- a/lib/resolve.c +++ b/lib/resolve.c @@ -853,6 +853,8 @@ int kr_resolve_consume(struct kr_request *request, struct kr_transport **transpo if (transport && !qry->flags.CACHED) { if (!(request->state & KR_STATE_FAIL)) { /* Do not complete NS address resolution on soft-fail. */ + if (kr_fails_assert(packet->wire)) + return KR_STATE_FAIL; const int rcode = knot_wire_get_rcode(packet->wire); if (rcode != KNOT_RCODE_SERVFAIL && rcode != KNOT_RCODE_REFUSED) { qry->flags.AWAIT_IPV6 = false; @@ -886,7 +888,7 @@ int kr_resolve_consume(struct kr_request *request, struct kr_transport **transpo } /* Pop query if resolved. */ - if (request->state == KR_STATE_YIELD) { + if (request->state == KR_STATE_YIELD) { // NOLINT(bugprone-branch-clone) return KR_STATE_PRODUCE; /* Requery */ } else if (qry->flags.RESOLVED) { kr_rplan_pop(rplan, qry); @@ -1051,7 +1053,7 @@ static int forward_trust_chain_check(struct kr_request *request, struct kr_query /* set `nods` */ if ((qry->stype == KNOT_RRTYPE_DS) && - knot_dname_is_equal(wanted_name, qry->sname)) { + knot_dname_is_equal(wanted_name, qry->sname)) { // NOLINT(bugprone-branch-clone) nods = true; } else if (resume && !ds_req) { nods = false; @@ -1612,6 +1614,7 @@ int kr_resolve_finish(struct kr_request *request, int state) knot_wire_clear_ad(wire); knot_wire_clear_aa(wire); knot_wire_set_rcode(wire, KNOT_RCODE_SERVFAIL); + default:; // Do nothing } } } diff --git a/lib/selection.c b/lib/selection.c index c25782e62..cce5d426c 100644 --- a/lib/selection.c +++ b/lib/selection.c @@ -149,7 +149,7 @@ struct rtt_state get_rtt_state(const uint8_t *ip, size_t len, knot_db_val_t key = cache_key(ip, len); - if (cache->api->read(db, stats, &key, &value, 1)) { + if (cache->api->read(db, stats, &key, &value, 1)) { // NOLINT(bugprone-branch-clone) state = default_rtt_state; } else if (kr_fails_assert(value.len == sizeof(struct rtt_state))) { // shouldn't happen but let's be more robust diff --git a/lib/utils.c b/lib/utils.c index 8b7e12709..2a0635e02 100644 --- a/lib/utils.c +++ b/lib/utils.c @@ -921,9 +921,8 @@ int kr_ranked_rrarray_add(ranked_rr_array_t *array, const knot_rrset_t *rr, static int rdata_p_cmp(const void *rp1, const void *rp2) { /* Just correct types of the parameters and pass them dereferenced. */ - const knot_rdata_t - *const *r1 = rp1, - *const *r2 = rp2; + const knot_rdata_t *const *r1 = (const knot_rdata_t *const *)rp1; + const knot_rdata_t *const *r2 = (const knot_rdata_t *const *)rp2; return knot_rdata_cmp(*r1, *r2); } int kr_ranked_rrarray_finalize(ranked_rr_array_t *array, uint32_t qry_uid, knot_mm_t *pool) @@ -948,7 +947,7 @@ int kr_ranked_rrarray_finalize(ranked_rr_array_t *array, uint32_t qry_uid, knot_ } else { /* Multiple RRs; first: sort the array. */ stashed->rr->additional = NULL; - qsort(ra->at, ra->len, sizeof(ra->at[0]), rdata_p_cmp); + qsort((void *)ra->at, ra->len, array_member_size(*ra), rdata_p_cmp); /* Prune duplicates: NULL all except the last instance. */ int dup_count = 0; for (int i = 0; i + 1 < ra->len; ++i) { diff --git a/modules/dnstap/dnstap.c b/modules/dnstap/dnstap.c index 757266728..07780fcbc 100644 --- a/modules/dnstap/dnstap.c +++ b/modules/dnstap/dnstap.c @@ -193,6 +193,7 @@ static int dnstap_log(kr_layer_t *ctx, enum dnstap_log_phase phase) { m.socket_family = DNSTAP__SOCKET_FAMILY__INET6; m.has_socket_family = true; break; + default:; } } diff --git a/modules/hints/hints.c b/modules/hints/hints.c index a3f2f3002..af05ee24e 100644 --- a/modules/hints/hints.c +++ b/modules/hints/hints.c @@ -194,9 +194,11 @@ static const knot_dname_t * raw_addr2reverse(const uint8_t *raw_addr, int family #undef REV_MAXLEN if (family == AF_INET) { - snprintf(reverse_addr, sizeof(reverse_addr), - "%d.%d.%d.%d.in-addr.arpa.", - raw_addr[3], raw_addr[2], raw_addr[1], raw_addr[0]); + int ret = snprintf(reverse_addr, sizeof(reverse_addr), + "%d.%d.%d.%d.in-addr.arpa.", + raw_addr[3], raw_addr[2], raw_addr[1], raw_addr[0]); + if (kr_fails_assert(ret > 0 && ret <= sizeof(reverse_addr))) + return NULL; } else if (family == AF_INET6) { char *ra_it = reverse_addr; for (int i = 15; i >= 0; --i) { @@ -262,7 +264,10 @@ static int add_reverse_pair(struct kr_zonecut *hints, const char *name, const ch return kr_error(EINVAL); } - return kr_zonecut_add(hints, key, ptr_name, knot_dname_size(ptr_name)); + size_t dname_size = knot_dname_size(ptr_name); + if (kr_fails_assert(dname_size < INT_MAX)) + return kr_error(EINVAL); + return kr_zonecut_add(hints, key, ptr_name, (int)dname_size); } /** For a given name, remove either one address or all of them (if == NULL). @@ -276,7 +281,9 @@ static int del_pair(struct hints_data *data, const char *name, const char *addr) if (!knot_dname_from_str(key, name, sizeof(key))) { return kr_error(EINVAL); } - int key_len = knot_dname_size(key); + size_t key_len = knot_dname_size(key); + if (kr_fails_assert(key_len <= INT_MAX)) + return kr_error(EINVAL); if (addr) { /* Remove the pair. */ @@ -286,7 +293,7 @@ static int del_pair(struct hints_data *data, const char *name, const char *addr) } const knot_dname_t *reverse_key = addr2reverse(addr); - kr_zonecut_del(&data->reverse_hints, reverse_key, key, key_len); + kr_zonecut_del(&data->reverse_hints, reverse_key, key, (int)key_len); return kr_zonecut_del(&data->hints, key, kr_inaddr(&ia.ip), kr_inaddr_len(&ia.ip)); } @@ -306,7 +313,7 @@ static int del_pair(struct hints_data *data, const char *name, const char *addr) ? AF_INET : AF_INET6; const knot_dname_t *reverse_key = raw_addr2reverse(addr_val, family); if (reverse_key != NULL) { - kr_zonecut_del(&data->reverse_hints, reverse_key, key, key_len); + kr_zonecut_del(&data->reverse_hints, reverse_key, key, (int)key_len); } } diff --git a/modules/stats/stats.c b/modules/stats/stats.c index ebb287783..129023f83 100644 --- a/modules/stats/stats.c +++ b/modules/stats/stats.c @@ -116,7 +116,7 @@ static inline int collect_key(char *key, const knot_dname_t *name, uint16_t type if (key_len < 0) { return kr_error(key_len); } - return key_len + sizeof(type); + return key_len + (int)sizeof(type); } static void collect_sample(struct stat_data *data, struct kr_rplan *rplan) @@ -313,26 +313,26 @@ static char* stats_get(void *env, struct kr_module *module, const char *args) struct stat_data *data = module->data; /* Expecting CHAR_BIT to be 8, this is a safe bet */ - char *ret = malloc(3 * sizeof(size_t) + 2); - if (!ret) { - return NULL; - } + char *str_value = NULL; + int ret = 0; /* Check if it exists in const map. */ for (unsigned i = 0; i < metric_const_end; ++i) { if (strcmp(const_metrics[i].key, args) == 0) { - sprintf(ret, "%zu", const_metrics[i].val); - return ret; + ret = asprintf(&str_value, "%zu", const_metrics[i].val); + if (ret < 0) + return NULL; + return str_value; } } /* Check in variable map */ trie_val_t *val = trie_get_try(data->trie, args, strlen(args)); - if (!val) { - free(ret); + if (!val) return NULL; - } - sprintf(ret, "%zu", (size_t) *val); - return ret; + ret = asprintf(&str_value, "%zu", (size_t) *val); + if (ret < 0) + return NULL; + return str_value; } /** Checks whether: @@ -356,9 +356,9 @@ static int list_entry(const char *key, uint32_t key_len, trie_val_t *val, void * struct list_entry_context *ctx = baton; if (!key_matches_prefix(key, key_len, ctx->key_prefix, ctx->key_prefix_len)) return 0; - size_t number = (size_t) *val; + size_t number = (size_t)*val; auto_free char *key_nt = strndup(key, key_len); - json_append_member(ctx->root, key_nt, json_mknumber(number)); + json_append_member(ctx->root, key_nt, json_mknumber((double)number)); return 0; } @@ -375,7 +375,7 @@ static char* stats_list(void *env, struct kr_module *module, const char *args) for (unsigned i = 0; i < metric_const_end; ++i) { struct const_metric_elm *elm = &const_metrics[i]; if (!args || strncmp(elm->key, args, args_len) == 0) { - json_append_member(root, elm->key, json_mknumber(elm->val)); + json_append_member(root, elm->key, json_mknumber((double)elm->val)); } } struct list_entry_context ctx = { diff --git a/utils/cache_gc/categories.c b/utils/cache_gc/categories.c index 19dec45c5..aaa1ae538 100644 --- a/utils/cache_gc/categories.c +++ b/utils/cache_gc/categories.c @@ -18,7 +18,7 @@ static bool rrtype_is_infrastructure(uint16_t r) } } -static int get_random(int to) +static unsigned int get_random(int to) { // We don't need these to be really unpredictable, // but this should be cheap enough not to be noticeable. diff --git a/utils/cache_gc/db.c b/utils/cache_gc/db.c index fc4a2fdba..c31ff220d 100644 --- a/utils/cache_gc/db.c +++ b/utils/cache_gc/db.c @@ -9,11 +9,13 @@ #include #include +#define MDB_FILE "/data.mdb" + int kr_gc_cache_open(const char *cache_path, struct kr_cache *kres_db, knot_db_t ** libknot_db) { - char cache_data[strlen(cache_path) + 10]; - snprintf(cache_data, sizeof(cache_data), "%s/data.mdb", cache_path); + char cache_data[strlen(cache_path) + sizeof(MDB_FILE)]; + (void)snprintf(cache_data, sizeof(cache_data), "%s" MDB_FILE, cache_path); struct stat st = { 0 }; if (stat(cache_path, &st) || !(st.st_mode & S_IFDIR) diff --git a/utils/cache_gc/kr_cache_gc.c b/utils/cache_gc/kr_cache_gc.c index 5978345c7..62465f51d 100644 --- a/utils/cache_gc/kr_cache_gc.c +++ b/utils/cache_gc/kr_cache_gc.c @@ -194,12 +194,12 @@ int kr_cache_gc(kr_cache_gc_cfg_t *cfg, kr_cache_gc_state_t **state) // Mixing ^^ page usage and entry sizes (key+value lengths) didn't work // too well, probably due to internal fragmentation after some GC cycles. // Therefore let's scale this by the ratio of these two sums. - ssize_t cats_sumsize = 0; + size_t cats_sumsize = 0; for (int i = 0; i < CATEGORIES; ++i) { cats_sumsize += cats.categories_sizes[i]; } /* use less precise variant to avoid 32-bit overflow */ - ssize_t amount_tofree = cats_sumsize / 100 * cfg->cache_to_be_freed; + size_t amount_tofree = cats_sumsize / 100 * cfg->cache_to_be_freed; kr_log_debug(CACHE, "tofree: %zd / %zd\n", amount_tofree, cats_sumsize); if (VERBOSE_STATUS) { @@ -212,8 +212,11 @@ int kr_cache_gc(kr_cache_gc_cfg_t *cfg, kr_cache_gc_state_t **state) } category_t limit_category = CATEGORIES; - while (limit_category > 0 && amount_tofree > 0) { - amount_tofree -= cats.categories_sizes[--limit_category]; + while (limit_category > 0) { + size_t cat_size = cats.categories_sizes[--limit_category]; + if (cat_size > amount_tofree) + break; + amount_tofree -= cat_size; } printf("Cache analyzed in %.0lf msecs, %zu records, limit category is %d.\n", diff --git a/utils/cache_gc/main.c b/utils/cache_gc/main.c index 5adf19f06..fe131cd0a 100644 --- a/utils/cache_gc/main.c +++ b/utils/cache_gc/main.c @@ -13,6 +13,7 @@ #include "kr_cache_gc.h" static volatile int killed = 0; +static volatile int exit_code = 0; static void got_killed(int signum) { @@ -21,12 +22,10 @@ static void got_killed(int signum) case 1: break; case 2: - exit(5); + exit_code = 5; break; - case 3: - abort(); default: - kr_assert(false); + abort(); } } @@ -60,16 +59,20 @@ int main(int argc, char *argv[]) { printf("Knot Resolver Cache Garbage Collector, version %s\n", PACKAGE_VERSION); if (setvbuf(stdout, NULL, _IONBF, 0) || setvbuf(stderr, NULL, _IONBF, 0)) { - fprintf(stderr, "Failed to to set output buffering (ignored): %s\n", + (void)fprintf(stderr, "Failed to to set output buffering (ignored): %s\n", strerror(errno)); - fflush(stderr); + (void)fflush(stderr); } - signal(SIGTERM, got_killed); - signal(SIGKILL, got_killed); - signal(SIGPIPE, got_killed); - signal(SIGCHLD, got_killed); - signal(SIGINT, got_killed); + struct sigaction act = { + .sa_handler = got_killed, + .sa_flags = SA_RESETHAND, + }; + sigemptyset(&act.sa_mask); + kr_assert(!sigaction(SIGTERM, &act, NULL)); + kr_assert(!sigaction(SIGPIPE, &act, NULL)); + kr_assert(!sigaction(SIGCHLD, &act, NULL)); + kr_assert(!sigaction(SIGINT, &act, NULL)); kr_cache_gc_cfg_t cfg = { .ro_txn_items = 200, @@ -131,7 +134,6 @@ int main(int argc, char *argv[]) return 1; } - int exit_code = 0; kr_cache_gc_state_t *gc_state = NULL; bool last_espace = false; do { diff --git a/utils/client/.clang-tidy b/utils/client/.clang-tidy new file mode 100644 index 000000000..46c666ca0 --- /dev/null +++ b/utils/client/.clang-tidy @@ -0,0 +1,2 @@ +--- +Checks: '-*'