Skip to content

Commit

Permalink
logging: Disallow category args to LogInfo/Warning/Error
Browse files Browse the repository at this point in the history
  • Loading branch information
@ryanofsky
ryanofsky committed Dec 5, 2024
1 parent 5db1958 commit 0b5cbf9
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 27 deletions.
46 changes: 31 additions & 15 deletions src/logging.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -280,12 +280,28 @@ 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>
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>
static inline BCLog::Source _LogSource(std::string_view fmt) { return {}; }

//! 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
//! 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)
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!");
}

//! Internal helper to format log arguments and call a logging function.
template <typename LogFn, typename Source, typename SourceArg, typename... Args>
requires (!std::is_convertible_v<SourceArg, std::string_view>)
Expand All @@ -306,15 +322,15 @@ 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, ...) \
do { \
if (LogEnabled(_LogSource(_FirstArg((__VA_ARGS__))), (level))) { \
const auto& func = __func__; \
_LogFormat([&](auto&& source, auto&& message) { \
source.logger.LogPrintStr(message, func, __FILE__, \
__LINE__, source.category, (level)); \
}, _LogSource(_FirstArg((__VA_ARGS__))), __VA_ARGS__); \
} \
#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__); \
} \
} while (0)

//! Logging macros which output log messages at the specified levels, and avoid
Expand Down Expand Up @@ -349,12 +365,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, __VA_ARGS__)
#define LogWarning(...) _LogPrint(BCLog::Level::Warning, __VA_ARGS__)
#define LogInfo(...) _LogPrint(BCLog::Level::Info, __VA_ARGS__)
#define LogDebug(...) _LogPrint(BCLog::Level::Debug, __VA_ARGS__)
#define LogTrace(...) _LogPrint(BCLog::Level::Trace, __VA_ARGS__)
#define LogPrintLevel(source, level, ...) _LogPrint(level, source, __VA_ARGS__)
#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__)

//! Deprecated macros.
#define LogPrintf(...) LogInfo(__VA_ARGS__)
Expand Down
18 changes: 6 additions & 12 deletions src/test/logging_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,14 +124,14 @@ BOOST_FIXTURE_TEST_CASE(logging_source_args, LogSetup)
LogTrace("trace %s\n", "arg");

// Test logging with category arguments.
LogError(BCLog::NET, "error\n");
LogWarning(BCLog::NET, "warning\n");
LogInfo(BCLog::NET, "info\n");
// 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!
LogDebug(BCLog::NET, "debug\n");
LogTrace(BCLog::NET, "trace\n");
LogError(BCLog::NET, "error %s\n", "arg");
LogWarning(BCLog::NET, "warning %s\n", "arg");
LogInfo(BCLog::NET, "info %s\n", "arg");
// 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!
LogDebug(BCLog::NET, "debug %s\n", "arg");
LogTrace(BCLog::NET, "trace %s\n", "arg");

Expand Down Expand Up @@ -161,15 +161,9 @@ BOOST_FIXTURE_TEST_CASE(logging_source_args, LogSetup)
"[debug] debug arg",
"[trace] trace arg",

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

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

Expand Down

0 comments on commit 0b5cbf9

Please sign in to comment.