refactor: Allow NULL logger; make it no-op in NDEBUG.

.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

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)
 

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
 

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 \

other/bootstrap_daemon/docker/tox-bootstrapd.sha256

@@ -1 +1 @@
-60f76a2186014870804b8dbacc16c68e8b019e02699584d007327a72de9e53de  /usr/local/bin/tox-bootstrapd
+4a2bfe7abf6060f252154062818541e485344b350cf975f7aeea78ff249bfa65  /usr/local/bin/tox-bootstrapd

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 "<unknown>";
 }
+#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) {

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, ...);