From e4d1958ffaae80ff84029ba2e44b0579f9f2e668 Mon Sep 17 00:00:00 2001 From: iphydf Date: Sun, 3 Apr 2022 14:52:26 +0000 Subject: [PATCH] refactor: Allow NULL logger; make it no-op in NDEBUG. --- .github/scripts/flags-clang.sh | 2 ++ CMakeLists.txt | 5 ----- INSTALL.md | 1 - other/analysis/run-clang | 1 + .../docker/tox-bootstrapd.sha256 | 2 +- toxcore/logger.c | 13 ++++++------- toxcore/logger.h | 17 ++++++++--------- 7 files changed, 18 insertions(+), 23 deletions(-) diff --git a/.github/scripts/flags-clang.sh b/.github/scripts/flags-clang.sh index afa842ae..dc47c648 100644 --- a/.github/scripts/flags-clang.sh +++ b/.github/scripts/flags-clang.sh @@ -26,6 +26,8 @@ add_flag -Wno-format-nonliteral # write {{{0}}} in some cases, which is ugly and a maintenance burden. add_flag -Wno-missing-braces add_flag -Wno-missing-field-initializers +# We don't use this attribute. It appears in the non-NDEBUG stderr logger. +add_flag -Wno-missing-noreturn # Useful sometimes, but we accept padding in structs for clarity. # Reordering fields to avoid padding will reduce readability. add_flag -Wno-padded diff --git a/CMakeLists.txt b/CMakeLists.txt index 1d6993ad..83a51f08 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -106,11 +106,6 @@ if(NOT USE_IPV6) add_definitions(-DUSE_IPV6=0) endif() -option(USE_STDERR_LOGGER "Enable logging to stderr when the logger is NULL" OFF) -if(USE_STDERR_LOGGER) - add_definitions(-DUSE_STDERR_LOGGER=1) -endif() - option(BUILD_MISC_TESTS "Build additional tests and utilities" OFF) option(BUILD_FUN_UTILS "Build additional just for fun utilities" OFF) diff --git a/INSTALL.md b/INSTALL.md index a5a0106e..f54a161d 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -127,7 +127,6 @@ There are some options that are available to configure the build. | `STRICT_ABI` | Enforce strict ABI export in dynamic libraries. | ON or OFF | OFF | | `TEST_TIMEOUT_SECONDS` | Limit runtime of each test to the number of seconds specified. | Positive number or nothing (empty string). | Empty string. | | `USE_IPV6` | Use IPv6 in tests. | ON or OFF | ON | -| `USE_STDERR_LOGGER` | Enable logging to stderr when the logger is NULL. | ON or OFF | OFF | You can get this list of option using the following commands diff --git a/other/analysis/run-clang b/other/analysis/run-clang index f2c67913..c1846407 100755 --- a/other/analysis/run-clang +++ b/other/analysis/run-clang @@ -24,6 +24,7 @@ run() { -Wno-global-constructors \ -Wno-missing-braces \ -Wno-missing-field-initializers \ + -Wno-missing-noreturn \ -Wno-old-style-cast \ -Wno-padded \ -Wno-sign-compare \ diff --git a/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 b/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 index 2540958c..b38b7f57 100644 --- a/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 +++ b/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 @@ -1 +1 @@ -60f76a2186014870804b8dbacc16c68e8b019e02699584d007327a72de9e53de /usr/local/bin/tox-bootstrapd +4a2bfe7abf6060f252154062818541e485344b350cf975f7aeea78ff249bfa65 /usr/local/bin/tox-bootstrapd diff --git a/toxcore/logger.c b/toxcore/logger.c index cbcf4e27..d281a660 100644 --- a/toxcore/logger.c +++ b/toxcore/logger.c @@ -22,7 +22,7 @@ struct Logger { void *userdata; }; -#ifdef USE_STDERR_LOGGER +#ifndef NDEBUG static const char *logger_level_name(Logger_Level level) { switch (level) { @@ -44,13 +44,18 @@ static const char *logger_level_name(Logger_Level level) return ""; } +#endif non_null(1, 3, 5, 6) nullable(7) static void logger_stderr_handler(void *context, Logger_Level level, const char *file, int line, const char *func, const char *message, void *userdata) { +#ifndef NDEBUG // GL stands for "global logger". fprintf(stderr, "[GL] %s %s:%d(%s): %s\n", logger_level_name(level), file, line, func, message); + fprintf(stderr, "Default stderr logger triggered; aborting program\n"); + abort(); +#endif } static const Logger logger_stderr = { @@ -58,7 +63,6 @@ static const Logger logger_stderr = { nullptr, nullptr, }; -#endif /* * Public Functions @@ -85,12 +89,7 @@ void logger_write(const Logger *log, Logger_Level level, const char *file, int l const char *format, ...) { if (log == nullptr) { -#ifdef USE_STDERR_LOGGER log = &logger_stderr; -#else - fprintf(stderr, "NULL logger not permitted.\n"); - abort(); -#endif } if (log->callback == nullptr) { diff --git a/toxcore/logger.h b/toxcore/logger.h index b11c738c..ee5838ae 100644 --- a/toxcore/logger.h +++ b/toxcore/logger.h @@ -53,17 +53,16 @@ void logger_kill(Logger *log); non_null(1) nullable(2, 3, 4) void logger_callback_log(Logger *log, logger_cb *function, void *context, void *userdata); -/** - * Main write function. If logging is disabled, this does nothing. +/** @brief Main write function. If logging is disabled, this does nothing. * - * If the logger is NULL, this writes to stderr. This behaviour should not be - * used in production code, but can be useful for temporarily debugging a - * function that does not have a logger available. It's essentially - * fprintf(stderr, ...), but with timestamps and source location. Toxcore must - * be built with -DUSE_STDERR_LOGGER for this to work. It will cause an - * assertion failure otherwise. + * If the logger is NULL and `NDEBUG` is not defined, this writes to stderr. + * This behaviour should not be used in production code, but can be useful for + * temporarily debugging a function that does not have a logger available. It's + * essentially `fprintf(stderr, ...)`, but with source location. + * + * If `NDEBUG` is defined, the NULL logger does nothing. */ -non_null() GNU_PRINTF(6, 7) +non_null(3, 5, 6) nullable(1) GNU_PRINTF(6, 7) void logger_write( const Logger *log, Logger_Level level, const char *file, int line, const char *func, const char *format, ...);