Skip to content

Commit aecba18

Browse files
authored
Added a nullptr check in macro-free log functions (#895)
1 parent 25bc0b7 commit aecba18

File tree

4 files changed

+42
-1
lines changed

4 files changed

+42
-1
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,8 @@
9999
## v11.1.0
100100

101101
- Fixed thread-local context duplication across shared libraries ([#890](https://github.com/odygrd/quill/issues/890))
102+
- Added a `nullptr` check in macro-free log functions (`LogFunctions.h`) to allow calls with an uninitialized logger.
103+
Macro-based logging remains deliberately unchanged ([#894](https://github.com/odygrd/quill/issues/894))
102104

103105
## v11.0.2
104106

docs/macro_free_mode.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ The macro-free approach comes with performance implications:
2020

2121
4. **Backend Thread Impact**: Reduced throughput in the backend due to runtime metadata storage and processing
2222

23+
5. **Additional Safety Checks**: The macro-free functions perform a runtime check for `nullptr` logger. This can be useful in cases where a user might forget to initialize a logger or intentionally wants logging calls to become no-ops when the logger is null. (Note that there are also multiple ways to achieve no-op logging with macros, e.g., by setting the log level to `None`)
24+
2325
For performance-critical logging paths, the macro-based logging interface is recommended.
2426

2527
Available Functions

include/quill/LogFunctions.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,11 @@ template <typename TLogger, typename... Args>
325325
QUILL_ATTRIBUTE_HOT void log(TLogger* logger, char const* tags, LogLevel log_level, char const* fmt,
326326
SourceLocation const& location, Args&&... args)
327327
{
328+
if (QUILL_UNLIKELY(!logger))
329+
{
330+
return;
331+
}
332+
328333
static constexpr MacroMetadata macro_metadata{
329334
"[placeholder]", "[placeholder]", "[placeholder]",
330335
nullptr, LogLevel::None, MacroMetadata::Event::LogWithRuntimeMetadataHybridCopy};

test/integration_tests/MacroFreeMultiFrontendThreadsTest.cpp

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,33 @@ TEST_CASE("macro_free_multi_frontend_threads")
6060
});
6161
}
6262

63+
// wait all threads to join
6364
for (auto& elem : threads)
6465
{
6566
elem.join();
6667
}
6768

69+
// log from main thread - get logger from the first thread
70+
Logger* t0_logger = Frontend::get_logger(logger_name_prefix + std::to_string(0));
71+
REQUIRE(t0_logger);
72+
73+
t0_logger->set_log_level(quill::LogLevel::TraceL3);
74+
75+
tracel3(t0_logger, "a trace_l3 message");
76+
tracel2(t0_logger, "a trace_l2 message");
77+
tracel1(t0_logger, "a trace_l1 message");
78+
debug(t0_logger, "a debug message");
79+
info(t0_logger, "an info message");
80+
notice(t0_logger, "a notice message");
81+
warning(t0_logger, "a warning message");
82+
error(t0_logger, "an error message");
83+
critical(t0_logger, "a critical message");
84+
85+
// with nullptr
86+
Logger* null_logger = nullptr;
87+
info(null_logger, "null");
88+
warning(null_logger, "null");
89+
6890
// flush all log and remove all loggers
6991
for (Logger* logger : Frontend::get_all_loggers())
7092
{
@@ -77,7 +99,7 @@ TEST_CASE("macro_free_multi_frontend_threads")
7799

78100
// Read file and check
79101
std::vector<std::string> const file_contents = quill::testing::file_contents(filename);
80-
REQUIRE_EQ(file_contents.size(), number_of_messages * number_of_threads);
102+
REQUIRE_EQ(file_contents.size(), number_of_messages * number_of_threads + 9);
81103

82104
for (size_t i = 0; i < number_of_threads; ++i)
83105
{
@@ -91,5 +113,15 @@ TEST_CASE("macro_free_multi_frontend_threads")
91113
}
92114
}
93115

116+
REQUIRE(testing::file_contains(file_contents, "LOG_TRACE_L3 logger_0 a trace_l3 message"));
117+
REQUIRE(testing::file_contains(file_contents, "LOG_TRACE_L2 logger_0 a trace_l2 message"));
118+
REQUIRE(testing::file_contains(file_contents, "LOG_TRACE_L1 logger_0 a trace_l1 message"));
119+
REQUIRE(testing::file_contains(file_contents, "LOG_DEBUG logger_0 a debug message"));
120+
REQUIRE(testing::file_contains(file_contents, "LOG_INFO logger_0 an info message"));
121+
REQUIRE(testing::file_contains(file_contents, "LOG_NOTICE logger_0 a notice message"));
122+
REQUIRE(testing::file_contains(file_contents, "LOG_WARNING logger_0 a warning message"));
123+
REQUIRE(testing::file_contains(file_contents, "LOG_ERROR logger_0 an error message"));
124+
REQUIRE(testing::file_contains(file_contents, "LOG_CRITICAL logger_0 a critical message"));
125+
94126
testing::remove_file(filename);
95127
}

0 commit comments

Comments
 (0)