Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Apr 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/scripts/flags-clang.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 0 additions & 5 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
1 change: 0 additions & 1 deletion INSTALL.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions other/analysis/run-clang
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
2 changes: 1 addition & 1 deletion other/bootstrap_daemon/docker/tox-bootstrapd.sha256
Original file line number Diff line number Diff line change
@@ -1 +1 @@
60f76a2186014870804b8dbacc16c68e8b019e02699584d007327a72de9e53de /usr/local/bin/tox-bootstrapd
4a2bfe7abf6060f252154062818541e485344b350cf975f7aeea78ff249bfa65 /usr/local/bin/tox-bootstrapd
13 changes: 6 additions & 7 deletions toxcore/logger.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -44,21 +44,25 @@ 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 = {
logger_stderr_handler,
nullptr,
nullptr,
};
#endif

/*
* Public Functions
Expand All @@ -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) {
Expand Down
17 changes: 8 additions & 9 deletions toxcore/logger.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 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 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 `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, ...);
Expand Down