]> git.ipfire.org Git - thirdparty/knot-resolver.git/commitdiff
lib/utils: define kr_require(), kr_assume() macros
authorTomas Krizek <tomas.krizek@nic.cz>
Wed, 3 Mar 2021 14:45:59 +0000 (15:45 +0100)
committerTomas Krizek <tomas.krizek@nic.cz>
Tue, 25 May 2021 10:44:31 +0000 (12:44 +0200)
These macros should replace the use of assert() in our entire codebase.
assert() have the following issues:
  - can be turned off at compilation time
  - they don't have consistent meaning in our code

kr_require() behaves similarly to assert - it checks a condition and
aborts if it fails. Unlike asserts, these aren't turned off by using
-DNDEBUG. kr_require() should be used for non-recoverable errors.

kr_assume() is a way to check for non-fatal errors which supports error
reporing, debugging and recovery. The function returns a boolean value
which the caller must use for error handling. An error log message is
produced when the condition fails. Optionally, when kr_debug_assumption
is set to true, the process will use fork() and the child will abort().
This generates a coredump for debugging purposes, while allowing the
parent process to keep running and recover from the non-fatal error.
This can be useful for debugging hard to reproduce errors in production
environments.

daemon/main.c
lib/utils.c
lib/utils.h

index 8bbd5609b844733da86a9a82239eb4c419971131..78aec5ae59ba5cabcf95272e3e5b9cbbe4ddb12b 100644 (file)
@@ -26,6 +26,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <sys/resource.h>
+#include <sys/wait.h>
 #include <unistd.h>
 
 #if ENABLE_CAP_NG
@@ -44,8 +45,20 @@ struct args the_args_value;  /** Static allocation for the_args singleton. */
 
 static void signal_handler(uv_signal_t *handle, int signum)
 {
-       uv_stop(uv_default_loop());
-       uv_signal_stop(handle);
+       switch (signum) {
+       case SIGINT:  /* Fallthrough. */
+       case SIGTERM:
+               uv_stop(uv_default_loop());
+               uv_signal_stop(handle);
+               break;
+       case SIGCHLD:
+               /* Wait for all dead processes. */
+               while (waitpid(-1, NULL, WNOHANG) > 0);
+               break;
+       default:
+               kr_log_error("unhandled signal: %d\n", signum);
+               break;
+       }
 }
 
 /** SIGBUS -> attempt to remove the overflowing cache file and abort. */
@@ -502,11 +515,13 @@ int main(int argc, char **argv)
 
        uv_loop_t *loop = uv_default_loop();
        /* Catch some signals. */
-       uv_signal_t sigint, sigterm;
+       uv_signal_t sigint, sigterm, sigchld;
        if (true) ret = uv_signal_init(loop, &sigint);
        if (!ret) ret = uv_signal_init(loop, &sigterm);
+       if (!ret) ret = uv_signal_init(loop, &sigchld);
        if (!ret) ret = uv_signal_start(&sigint, signal_handler, SIGINT);
        if (!ret) ret = uv_signal_start(&sigterm, signal_handler, SIGTERM);
+       if (!ret) ret = uv_signal_start(&sigchld, signal_handler, SIGCHLD);
        /* Block SIGPIPE; see https://github.com/libuv/libuv/issues/45 */
        if (!ret && signal(SIGPIPE, SIG_IGN) == SIG_ERR) ret = errno;
        if (!ret) {
index 4aad3051465cf6c6cdabe244f77f91668a298535..bff6ed26cafe51ec30cb8eb14d8dc04e08af333c 100644 (file)
 
 /* Logging & debugging */
 bool kr_verbose_status = false;
+bool kr_debug_assumption = true;
+
+void kr_fail(bool is_fatal, const char *expr, const char *func, const char *file, int line)
+{
+       if (is_fatal)
+               kr_log_critical("requirement \"%s\" failed in %s@%s:%d\n", expr, func, file, line);
+       else
+               kr_log_error("assumption \"%s\" failed in %s@%s:%d\n", expr, func, file, line);
+
+       if (is_fatal || (kr_debug_assumption && fork() == 0))
+               abort();
+}
 
 /*
  * Macros.
index cbd0403d8c653cb7f29e14a0e3ec00baa25260c0..5660186a7d2b53f2107dc576b142188f6fa7587a 100644 (file)
@@ -7,6 +7,7 @@
 #include <assert.h>
 #include <dirent.h>
 #include <stdio.h>
+#include <stdlib.h>
 #include <stdbool.h>
 #include <sys/socket.h>
 #include <sys/time.h>
@@ -43,8 +44,41 @@ typedef void (*trace_log_f)(const struct kr_request *request, const char *msg);
 
 #define kr_log_info printf
 #define kr_log_error(...) fprintf(stderr, ## __VA_ARGS__)
+#define kr_log_critical(...) kr_log_error(__VA_ARGS__)
 #define kr_log_deprecate(...) fprintf(stderr, "deprecation WARNING: " __VA_ARGS__)
 
+/** assert() but always, regardless of -DNDEBUG.  See also kr_assume(). */
+#define kr_require(expression) do { if (!(expression)) { \
+               kr_fail(true, #expression, __func__, __FILE__, __LINE__); \
+               __builtin_unreachable(); /* aid code analysis */ \
+       } } while (false)
+
+/** Check an assumption that's recoverable.  Return the expression.
+ *
+ * If the check fails, optionally fork()+abort() to generate coredump
+ * and continue running in parent process.  Return value must be handled to
+ * ensure safe recovery from error.  Use kr_require() for unrecoverable checks.
+ */
+#define kr_assume(expression) kr_assume_func((expression), #expression,       \
+                                            __func__, __FILE__, __LINE__)
+
+/** Whether kr_assume() checks should result fork and abort. */
+KR_EXPORT extern bool kr_debug_assumption;
+
+/** Use kr_require() and kr_assume() instead of directly this function. */
+KR_EXPORT KR_COLD void kr_fail(bool is_fatal, const char* expr, const char *func,
+                               const char *file, int line);
+
+/** Use kr_require() and kr_assume() instead of directly this function. */
+__attribute__ ((warn_unused_result))
+static inline bool kr_assume_func(bool result, const char *expr, const char *func,
+                                 const char *file, int line)
+{
+       if (!result)
+               kr_fail(false, expr, func, file, line);
+       return result;
+}
+
 /* Always export these, but override direct calls by macros conditionally. */
 /** Whether in --verbose mode.  Only use this for reading. */
 KR_EXPORT extern bool kr_verbose_status;