Skip to content

Commit

Permalink
logging: Require category args to LogDebug/Trace again
Browse files Browse the repository at this point in the history
  • Loading branch information
@hodlinator
hodlinator committed Dec 14, 2024
1 parent 0b5cbf9 commit cebc9e2
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 43 deletions.
56 changes: 36 additions & 20 deletions src/logging.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -280,23 +280,36 @@ bool GetLogCategory(BCLog::LogFlags& flag, std::string_view str);
//! Internal helper to get log source object from macro argument, if argument is
//! BCLog::Source object or a category constant that BCLog::Source can be
//! constructed from.
template <bool forbid_category>
template <bool take_category>
static inline const BCLog::Source& _LogSource(const BCLog::Source& source LIFETIMEBOUND) { return source; }

//! Internal helper to get log source object from macro argument, if argument is
//! just a format string and no source or category was provided.
template <bool forbid_category>
template <bool take_category>
static inline BCLog::Source _LogSource(std::string_view fmt) { return {}; }

//! Internal helper to trigger a compile error if a caller forgets to pass a
//! category constant as a source argument to a logging macro that specifies
//! take_category == true. There is no technical reason why all logging macros
//! could not have the category argument be optional, but low priority level
//! macros should specify category in order to only be visible when the selected
//! category is explicitly enabled. This helps reduce log spam.
template <bool take_category>
requires (take_category)
static inline BCLog::Source _LogSource(std::string_view fmt)
{
static_assert(false, "Must pass category to Debug/Trace logging macros.");
}

//! Internal helper to trigger a compile error if a caller tries to pass a
//! category constant as a source argument to a logging macro that specifies
//! forbid_category == true. There is no technical reason why all logging
//! take_category == false. There is no technical reason why all logging
//! macros cannot accept category arguments, but for various reasons, such as
//! (1) not wanting to allow users filter by category at high priority levels,
//! and (2) wanting to incentivize developers to use lower log levels to avoid
//! log spam, passing category constants at higher levels is forbidden.
template <bool forbid_category>
requires (forbid_category)
template <bool take_category>
requires (!take_category)
static inline BCLog::Source _LogSource(BCLog::LogFlags category)
{
static_assert(false, "Cannot pass BCLog::LogFlags category argument to high level Info/Warning/Error logging macros. Please use lower level Debug/Trace macro, or drop the category argument!");
Expand All @@ -322,15 +335,18 @@ void _LogFormat(LogFn&& log, Source&& source, util::ConstevalFormatString<sizeof
#define _FirstArg(args) _FirstArgImpl args

//! Internal helper to check level and log. Avoids evaluating arguments if not logging.
#define _LogPrint(level, forbid_category, ...) \
do { \
if (LogEnabled(_LogSource<forbid_category>(_FirstArg((__VA_ARGS__))), (level))) { \
const auto& func = __func__; \
_LogFormat([&](auto&& source, auto&& message) { \
source.logger.LogPrintStr(message, func, __FILE__, \
__LINE__, source.category, (level)); \
}, _LogSource<forbid_category>(_FirstArg((__VA_ARGS__))), __VA_ARGS__); \
} \
#define _LogPrint(level, take_category, ...) \
do { \
auto log_source = _LogSource<take_category>(_FirstArg((__VA_ARGS__))); \
if (LogEnabled(log_source, (level))) { \
const auto& func = __func__; \
const auto log_category = \
take_category ? log_source.category : BCLog::ALL; \
_LogFormat([&](auto&& source, auto&& message) { \
source.logger.LogPrintStr(message, func, __FILE__, \
__LINE__, log_category, (level)); \
}, log_source, __VA_ARGS__); \
} \
} while (0)

//! Logging macros which output log messages at the specified levels, and avoid
Expand Down Expand Up @@ -365,12 +381,12 @@ void _LogFormat(LogFn&& log, Source&& source, util::ConstevalFormatString<sizeof
//!
//! Using source objects also allows diverting log messages to a local logger
//! instead of the global logging instance.
#define LogError(...) _LogPrint(BCLog::Level::Error, true, __VA_ARGS__)
#define LogWarning(...) _LogPrint(BCLog::Level::Warning, true, __VA_ARGS__)
#define LogInfo(...) _LogPrint(BCLog::Level::Info, true, __VA_ARGS__)
#define LogDebug(...) _LogPrint(BCLog::Level::Debug, false, __VA_ARGS__)
#define LogTrace(...) _LogPrint(BCLog::Level::Trace, false, __VA_ARGS__)
#define LogPrintLevel(source, level, ...) _LogPrint(level, false, source, __VA_ARGS__)
#define LogError(...) _LogPrint(BCLog::Level::Error, false, __VA_ARGS__)
#define LogWarning(...) _LogPrint(BCLog::Level::Warning, false, __VA_ARGS__)
#define LogInfo(...) _LogPrint(BCLog::Level::Info, false, __VA_ARGS__)
#define LogDebug(...) _LogPrint(BCLog::Level::Debug, true, __VA_ARGS__)
#define LogTrace(...) _LogPrint(BCLog::Level::Trace, true, __VA_ARGS__)
#define LogPrintLevel(source, level, ...) _LogPrint(level, true, source, __VA_ARGS__)

//! Deprecated macros.
#define LogPrintf(...) LogInfo(__VA_ARGS__)
Expand Down
42 changes: 19 additions & 23 deletions src/test/logging_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,9 @@ BOOST_AUTO_TEST_CASE(logging_local_logger)
LogTrace(log, "trace %s\n", "arg");

constexpr auto expected{std::to_array({
"[net:error] error arg\n",
"[net:warning] warning arg\n",
"[net:info] info arg\n",
"[error] error arg\n",
"[warning] warning arg\n",
"info arg\n",
"[net] debug arg\n",
"[net:trace] trace arg\n",
})};
Expand All @@ -115,23 +115,23 @@ BOOST_FIXTURE_TEST_CASE(logging_source_args, LogSetup)
LogError("error\n");
LogWarning("warning\n");
LogInfo("info\n");
LogDebug("debug\n");
LogTrace("trace\n");
// LogDebug("debug\n"); // Not allowed because of take_category!
// LogTrace("trace\n"); // Not allowed because of take_category!
LogError("error %s\n", "arg");
LogWarning("warning %s\n", "arg");
LogInfo("info %s\n", "arg");
LogDebug("debug %s\n", "arg");
LogTrace("trace %s\n", "arg");
// LogDebug("debug %s\n", "arg"); // Not allowed because of take_category!
// LogTrace("trace %s\n", "arg"); // Not allowed because of take_category!

// Test logging with category arguments.
// LogError(BCLog::NET, "error\n"); // Not allowed because of forbid_category!
// LogWarning(BCLog::NET, "warning\n"); // Not allowed because of forbid_category!
// LogInfo(BCLog::NET, "info\n"); // Not allowed because of forbid_category!
// LogError(BCLog::NET, "error\n"); // Not allowed because of take_category!
// LogWarning(BCLog::NET, "warning\n"); // Not allowed because of take_category!
// LogInfo(BCLog::NET, "info\n"); // Not allowed because of take_category!
LogDebug(BCLog::NET, "debug\n");
LogTrace(BCLog::NET, "trace\n");
// LogError(BCLog::NET, "error %s\n", "arg"); // Not allowed because of forbid_category!
// LogWarning(BCLog::NET, "warning %s\n", "arg"); // Not allowed because of forbid_category!
// LogInfo(BCLog::NET, "info %s\n", "arg"); // Not allowed because of forbid_category!
// LogError(BCLog::NET, "error %s\n", "arg"); // Not allowed because of take_category!
// LogWarning(BCLog::NET, "warning %s\n", "arg"); // Not allowed because of take_category!
// LogInfo(BCLog::NET, "info %s\n", "arg"); // Not allowed because of take_category!
LogDebug(BCLog::NET, "debug %s\n", "arg");
LogTrace(BCLog::NET, "trace %s\n", "arg");

Expand All @@ -152,30 +152,26 @@ BOOST_FIXTURE_TEST_CASE(logging_source_args, LogSetup)
"[error] error",
"[warning] warning",
"info",
"[debug] debug",
"[trace] trace",

"[error] error arg",
"[warning] warning arg",
"info arg",
"[debug] debug arg",
"[trace] trace arg",

"[net] debug",
"[net:trace] trace",

"[net] debug arg",
"[net:trace] trace arg",

"[tor:error] error",
"[tor:warning] warning",
"[tor:info] info",
"[error] error",
"[warning] warning",
"info",
"[tor] debug",
"[tor:trace] trace",

"[tor:error] error arg",
"[tor:warning] warning arg",
"[tor:info] info arg",
"[error] error arg",
"[warning] warning arg",
"info arg",
"[tor] debug arg",
"[tor:trace] trace arg",
})};
Expand Down

0 comments on commit cebc9e2

Please sign in to comment.