]> git.ipfire.org Git - thirdparty/knot-resolver.git/commitdiff
Silence Clang-Tidy
authorOto Šťáva <oto.stava@nic.cz>
Mon, 29 Apr 2024 13:09:01 +0000 (15:09 +0200)
committerOto Šťáva <oto.stava@nic.cz>
Mon, 13 May 2024 13:09:21 +0000 (15:09 +0200)
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.

34 files changed:
.clang-tidy
daemon/bindings/net.c
daemon/engine.c
daemon/io.c
daemon/main.c
daemon/proxyv2.c
daemon/session.c
daemon/tls.c
daemon/tls.h
daemon/tls_ephemeral_credentials.c
daemon/tls_session_ticket-srv.c
daemon/udp_queue.c
daemon/worker.c
daemon/zimport.c
lib/cache/peek.c
lib/dnssec.c
lib/generic/array.h
lib/generic/lru.h
lib/generic/queue.c
lib/generic/queue.h
lib/generic/trie.c
lib/layer/validate.c
lib/log.c
lib/resolve.c
lib/selection.c
lib/utils.c
modules/dnstap/dnstap.c
modules/hints/hints.c
modules/stats/stats.c
utils/cache_gc/categories.c
utils/cache_gc/db.c
utils/cache_gc/kr_cache_gc.c
utils/cache_gc/main.c
utils/client/.clang-tidy [new file with mode: 0644]

index b496044c4f8e94b04f029e7ff4a93e0d6cbf8fdf..ecc9a62144cfedb7c833e1e9fe3dd20f63f9c80b 100644 (file)
@@ -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
index f1fa6f3a3301692f6b6b3fb70399892a8cf517cb..0075d0f2d4c21a76cff69ff64cd19acc317fb6fd 100644 (file)
@@ -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;
 
index 1f9f1005181ee1a684e9902470431c4574bc314a..8c00a5befdf351d1167a16385129c33f146a1bd3 100644 (file)
@@ -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"
index 6d548d7778c6c636d58be0c4c23e1a4e53dfc25c..9299ff2ad2178236e65342c2d2e13d6f3bde39d8 100644 (file)
@@ -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);
        }
 }
 
index 41a55ad525a8c207b51a61c9d288582b0d8cdb5e..a346a5c8394a529cdee52a37c872eecc67287cd5 100644 (file)
@@ -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);
        }
index aedbb91a70bf5c20018000487be3f86e55fb6ffa..819700320acb63a1877f099df8c87b54c9812aaf 100644 (file)
@@ -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 */
                }
        }
 
index ed0ff6869472cb37eb58a4c12852e01c44ea0c57..91d3c39e1946f5239a7b4b0190ce0dd22cb3a25e 100644 (file)
@@ -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
index 2e1631b97097fc0e887edbdbfa28b467f9b66045..0ab3968272a8624fb1739d4004631ba5421f7b8a 100644 (file)
@@ -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;
index af1f5c98e630f8de93fb6ef9ba2b9c3aaea0b39b..c30444bea1a43752d43f1558070e38986f619ec2 100644 (file)
@@ -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;
index ff4682f98503e5b414bf686a1fd0d4b1f813702e..0d9ec6db6453bb3685b66e853261b41dd27618e4 100644 (file)
 
 #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) {
index b198903005f4019eeeec165175905ed86e32a22d..26d41862f6179f27d1c481834a7b69042d2c326a 100644 (file)
@@ -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",
index 1f8ff39ecbbc2d9f6ced3825225bbfed0024656f..7ed600a9e86d029b1c391456ae7afdcdfb2d1744 100644 (file)
@@ -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))
index 8b6b49e67338d4ff709695aec6d23855eabdec32..12c08f160eb96066b501609a2eeaa7c79914c43b 100644 (file)
@@ -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)
index af21a159454a0b95251456d90699b205a07c793e..39799b604c5420f47198ba1c32d3e48a52c6bef5 100644 (file)
@@ -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;
index 191e85567d8adfbb7d51d22802252e1171030992..f0bb79cce16794d0b23517f545179dad4d7aaa72 100644 (file)
@@ -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*. */
index 12b8f20211d6ae06393c38496030d56d93d04136..eb4b33b1a0bb9869657a8732ea0c539edc2ed900 100644 (file)
@@ -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
index 9f351189d1181ed4c454a89a6e5b9d6914afb436..9bea546be1e55917cd19c925a4289b35b3ba8e75 100644 (file)
@@ -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
 
 /** @} */
index 448c1b92f35c3e1291a8b4796b618658e7c02e3e..1c1dd81ace9467d828a1286a2d91a86aab3b98ca 100644 (file)
 #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.
index 5bed153ef6b6a44f4a24f4912aabe026b81577ee..29609dd2e12652a008c35bf926e2023bc317246d 100644 (file)
@@ -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)
index 3fa52cea6b4dc7ee1716c9e8e8de22f408bad022..fc2a86f3c0df6e66c1759e18e2c316f5937a9089 100644 (file)
@@ -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. */
 #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.
index f9aceda799bc54e4480163a1738fd6a4760cfacf..21254eb4a085c3125609c8706ac3f4d727a0e080 100644 (file)
@@ -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;
index 3bdb205c4520c12c7cc744b35df84bd6718dd7e4..af20b2e45871d51c613c17ab6d70a5ac7d9675bc 100644 (file)
@@ -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;
index 9c8c7a6f338f90e2cbd0b14800dc3f4a73631305..9fb16e9100c283b83e597e1c77cad33bc2856926 100644 (file)
--- 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);
        }
 }
index 710b1ff2b8ade218a1ded3448ae77f4e7c4b4afd..d8198c34b2873fb9a1a318ee93ed1b24ce6ba383 100644 (file)
@@ -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
                        }
                }
        }
index c25782e62aafa9e06e06ddafe2efafdba74a7379..cce5d426c39326c8188f02ab98f02b46983db115 100644 (file)
@@ -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
index 8b7e12709e8353274acc6f44f63fe4fa7b3554d8..2a0635e026441f33fb9dde76a7e74dbb62488d78 100644 (file)
@@ -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) {
index 757266728d0459b0c67c6f247ce117f19bea91cf..07780fcbc6d7b226a610221a3e0e964a44c2a0ad 100644 (file)
@@ -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:;
                }
        }
 
index a3f2f3002e4ce88cc187cc82eafd9a412f02540b..af05ee24e3b0332c072eaeb9faf491d555bc282a 100644 (file)
@@ -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);
                }
        }
 
index ebb2877832eb95dd5454a6f74c5d2963c8f7d85e..129023f83f8c931e7c0c74d8bf7e1d17e8ef02c9 100644 (file)
@@ -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 = {
index 19dec45c5a0718e6a2c8f1d38ef5cf53e5c27c6e..aaa1ae5384ede678ef117d38c5da74a57c29fde6 100644 (file)
@@ -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.
index fc4a2fdbacd226b68527add07c6d9dacff8f49ca..c31ff220dcff093169632ecd554335aef3d36a47 100644 (file)
@@ -9,11 +9,13 @@
 #include <time.h>
 #include <sys/stat.h>
 
+#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)
index 5978345c79498c84ff6599a56b4665d4609335d9..62465f51ddf717ac6d75af6b71c1bec93b28eab6 100644 (file)
@@ -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",
index 5adf19f06ef9fd1a196f73a1d67921b3802e5b5b..fe131cd0a22274aacc40baa9f77293139538a13f 100644 (file)
@@ -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 (file)
index 0000000..46c666c
--- /dev/null
@@ -0,0 +1,2 @@
+---
+Checks: '-*'