Skip to content

Commit

Permalink
pw_log: Add the log module name to PW_HANDLE_LOG()
Browse files Browse the repository at this point in the history
pw_log backends previously were expected to access the
PW_LOG_MODULE_NAME macro to get the module name. Passing the log module
name to PW_HANDLE_LOG() instead makes it simple to use different log
module names for different lines in the same file. It also makes it
possible to set the log module name in a header file, where
PW_LOG_MODULE_NAME cannot be set without non-standard push/pop pragmas.

Fixes: b/254117152
Requires: pigweed-internal:37069
Change-Id: Ie50aa66e0d16c4dc21f1914c564ae0fc416d8c7a
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/120670
Reviewed-by: Armando Montanez <[email protected]>
Commit-Queue: Auto-Submit <[email protected]>
Commit-Queue: Wyatt Hepler <[email protected]>
  • Loading branch information
255 authored and CQ Bot Account committed Dec 22, 2022
1 parent a2674e1 commit 4c95c0b
Show file tree
Hide file tree
Showing 22 changed files with 286 additions and 168 deletions.
2 changes: 2 additions & 0 deletions pw_assert_log/assert_log.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,12 @@
extern "C" void pw_assert_HandleFailure(void) {
#if PW_ASSERT_ENABLE_DEBUG
PW_LOG(PW_LOG_LEVEL_FATAL,
"pw_assert_log",
PW_LOG_FLAGS,
"Crash: PW_ASSERT() or PW_DASSERT() failure");
#else
PW_LOG(PW_LOG_LEVEL_FATAL,
"pw_assert_log",
PW_LOG_FLAGS,
"Crash: PW_ASSERT() failure. Note: PW_DASSERT disabled");
#endif // PW_ASSERT_ENABLE_DEBUG
Expand Down
12 changes: 7 additions & 5 deletions pw_assert_log/public/pw_assert_log/assert_log.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,11 @@
// Forward directly to the backend PW_HANDLE_LOG() macro rather than PW_LOG().
// PW_LOG() checks the user-overridable PW_LOG_LEVEL macro, which may not work
// in headers without additional handling.
#define PW_ASSERT_HANDLE_FAILURE(condition_string) \
do { \
PW_HANDLE_LOG( \
PW_LOG_LEVEL_FATAL, PW_LOG_FLAGS, "Assert failed: " condition_string); \
PW_UNREACHABLE; \
#define PW_ASSERT_HANDLE_FAILURE(condition_string) \
do { \
PW_HANDLE_LOG(PW_LOG_LEVEL_FATAL, \
PW_LOG_MODULE_NAME, \
PW_LOG_FLAGS, \
"Assert failed: " condition_string); \
PW_UNREACHABLE; \
} while (0)
15 changes: 10 additions & 5 deletions pw_assert_log/public/pw_assert_log/check_log.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,14 @@
// Die with a message with several attributes included. This crash frontend
// funnels everything into the logger, which must then handle the true crash
// behaviour.
#define PW_HANDLE_CRASH(message, ...) \
do { \
PW_HANDLE_LOG( \
PW_LOG_LEVEL_FATAL, PW_LOG_FLAGS, "Crash: " message, __VA_ARGS__); \
PW_UNREACHABLE; \
#define PW_HANDLE_CRASH(message, ...) \
do { \
PW_HANDLE_LOG(PW_LOG_LEVEL_FATAL, \
PW_LOG_MODULE_NAME, \
PW_LOG_FLAGS, \
"Crash: " message, \
__VA_ARGS__); \
PW_UNREACHABLE; \
} while (0)

// Die with a message with several attributes included. This assert frontend
Expand All @@ -35,6 +38,7 @@
#define PW_HANDLE_ASSERT_FAILURE(condition_string, message, ...) \
do { \
PW_LOG(PW_LOG_LEVEL_FATAL, \
PW_LOG_MODULE_NAME, \
PW_LOG_FLAGS, \
"Check failed: " condition_string ". " message, \
__VA_ARGS__); \
Expand All @@ -58,6 +62,7 @@
message, ...) \
do { \
PW_LOG(PW_LOG_LEVEL_FATAL, \
PW_LOG_MODULE_NAME, \
PW_LOG_FLAGS, \
"Check failed: " \
arg_a_str " (=" type_fmt ") " \
Expand Down
2 changes: 2 additions & 0 deletions pw_assert_tokenized/log_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,14 @@ extern "C" void pw_assert_tokenized_HandleAssertFailure(
base64_buffer[len] = '\0';
#if PW_ASSERT_ENABLE_DEBUG
PW_LOG(PW_LOG_LEVEL_FATAL,
"pw_assert_tokenized",
PW_LOG_FLAGS,
"PW_ASSERT() or PW_DASSERT() failure at $%s:%d",
base64_buffer,
line_number);
#else
PW_LOG(PW_LOG_LEVEL_FATAL,
"pw_assert_tokenized",
PW_LOG_FLAGS,
"PW_ASSERT() failure. Note: PW_DASSERT disabled $%s:%d",
base64_buffer,
Expand Down
87 changes: 60 additions & 27 deletions pw_log/basic_log_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -72,33 +72,58 @@ TEST(BasicLog, CriticalLevel) {
}

TEST(BasicLog, ManualLevel) {
PW_LOG(PW_LOG_LEVEL_DEBUG, 0, "A manual DEBUG-level message");
PW_LOG(PW_LOG_LEVEL_DEBUG, 1, "A manual DEBUG-level message; with a flag");
PW_LOG(PW_LOG_LEVEL_DEBUG,
PW_LOG_MODULE_NAME,
0,
"A manual DEBUG-level message");
PW_LOG(PW_LOG_LEVEL_DEBUG,
PW_LOG_MODULE_NAME,
1,
"A manual DEBUG-level message; with a flag");

PW_LOG(PW_LOG_LEVEL_INFO, 0, "A manual INFO-level message");
PW_LOG(PW_LOG_LEVEL_INFO, 1, "A manual INFO-level message; with a flag");

PW_LOG(PW_LOG_LEVEL_WARN, 0, "A manual WARN-level message");
PW_LOG(PW_LOG_LEVEL_WARN, 1, "A manual WARN-level message; with a flag");

PW_LOG(PW_LOG_LEVEL_ERROR, 0, "A manual ERROR-level message");
PW_LOG(PW_LOG_LEVEL_ERROR, 1, "A manual ERROR-level message; with a flag");
PW_LOG(
PW_LOG_LEVEL_INFO, PW_LOG_MODULE_NAME, 0, "A manual INFO-level message");
PW_LOG(PW_LOG_LEVEL_INFO,
PW_LOG_MODULE_NAME,
1,
"A manual INFO-level message; with a flag");

PW_LOG(PW_LOG_LEVEL_CRITICAL, 0, "A manual CRITICAL-level message");
PW_LOG(
PW_LOG_LEVEL_CRITICAL, 1, "A manual CRITICAL-level message; with a flag");
PW_LOG_LEVEL_WARN, PW_LOG_MODULE_NAME, 0, "A manual WARN-level message");
PW_LOG(PW_LOG_LEVEL_WARN,
PW_LOG_MODULE_NAME,
1,
"A manual WARN-level message; with a flag");

PW_LOG(PW_LOG_LEVEL_ERROR,
PW_LOG_MODULE_NAME,
0,
"A manual ERROR-level message");
PW_LOG(PW_LOG_LEVEL_ERROR,
PW_LOG_MODULE_NAME,
1,
"A manual ERROR-level message; with a flag");

PW_LOG(PW_LOG_LEVEL_CRITICAL,
PW_LOG_MODULE_NAME,
0,
"A manual CRITICAL-level message");
PW_LOG(PW_LOG_LEVEL_CRITICAL,
PW_LOG_MODULE_NAME,
1,
"A manual CRITICAL-level message; with a flag");
}

TEST(BasicLog, FromAFunction) { LoggingFromFunction(); }

TEST(BasicLog, CustomLogLevels) {
// Log levels other than the standard ones work; what each backend does is
// implementation defined.
PW_LOG(0, 0, "Custom log level: 0");
PW_LOG(1, 0, "Custom log level: 1");
PW_LOG(2, 0, "Custom log level: 2");
PW_LOG(3, 0, "Custom log level: 3");
PW_LOG(100, 0, "Custom log level: 100");
PW_LOG(0, "", 0, "Custom log level: 0");
PW_LOG(1, "", 0, "Custom log level: 1");
PW_LOG(2, "", 0, "Custom log level: 2");
PW_LOG(3, "", 0, "Custom log level: 3");
PW_LOG(100, "", 0, "Custom log level: 100");
}

#define TEST_FAILED_LOG "IF THIS MESSAGE WAS LOGGED, THE TEST FAILED"
Expand All @@ -122,12 +147,18 @@ TEST(BasicLog, FilteringByFlags) {
#define PW_LOG_SKIP_LOGS_WITH_FLAGS 1

// Flag is set so these should all get zapped.
PW_LOG(PW_LOG_LEVEL_INFO, 1, TEST_FAILED_LOG);
PW_LOG(PW_LOG_LEVEL_ERROR, 1, TEST_FAILED_LOG);
PW_LOG(PW_LOG_LEVEL_INFO, PW_LOG_MODULE_NAME, 1, TEST_FAILED_LOG);
PW_LOG(PW_LOG_LEVEL_ERROR, PW_LOG_MODULE_NAME, 1, TEST_FAILED_LOG);

// However, a different flag bit should still log.
PW_LOG(PW_LOG_LEVEL_INFO, 1 << 1, "This flagged log is intended to appear");
PW_LOG(PW_LOG_LEVEL_ERROR, 1 << 1, "This flagged log is intended to appear");
PW_LOG(PW_LOG_LEVEL_INFO,
PW_LOG_MODULE_NAME,
1 << 1,
"This flagged log is intended to appear");
PW_LOG(PW_LOG_LEVEL_ERROR,
PW_LOG_MODULE_NAME,
1 << 1,
"This flagged log is intended to appear");

#undef PW_LOG_SKIP_LOGS_WITH_FLAGS
#define PW_LOG_SKIP_LOGS_WITH_FLAGS 0
Expand All @@ -141,7 +172,7 @@ TEST(BasicLog, ChangingTheModuleName) {
}

TEST(BasicLog, ShortNames) {
LOG(PW_LOG_LEVEL_INFO, 0, "Shrt lg");
LOG(PW_LOG_LEVEL_INFO, PW_LOG_MODULE_NAME, 0, "Shrt lg");
LOG_DEBUG("A debug log: %d", 1);
LOG_INFO("An info log: %d", 2);
LOG_WARN("A warning log: %d", 3);
Expand All @@ -150,7 +181,7 @@ TEST(BasicLog, ShortNames) {
}

TEST(BasicLog, UltraShortNames) {
LOG(PW_LOG_LEVEL_INFO, 0, "Shrt lg");
LOG(PW_LOG_LEVEL_INFO, PW_LOG_MODULE_NAME, 0, "Shrt lg");
DBG("A debug log: %d", 1);
INF("An info log: %d", 2);
WRN("A warning log: %d", 3);
Expand All @@ -167,15 +198,17 @@ TEST(BasicLog, FromPlainC) { BasicLogTestPlainC(); }
// functions tests fail to compile, because the arguments end up out-of-order.

#undef PW_LOG
#define PW_LOG(level, flags, message, ...) \
DoNothingFakeFunction("%d/%d/%d: incoming transmission [" message "]", \
#define PW_LOG(level, module, flags, message, ...) \
DoNothingFakeFunction(module, \
"%d/%d/%d: incoming transmission [" message "]", \
level, \
__LINE__, \
flags PW_COMMA_ARGS(__VA_ARGS__))

void DoNothingFakeFunction(const char*, ...) PW_PRINTF_FORMAT(1, 2);
void DoNothingFakeFunction(const char*, const char*, ...)
PW_PRINTF_FORMAT(2, 3);

void DoNothingFakeFunction(const char*, ...) {}
void DoNothingFakeFunction(const char*, const char*, ...) {}

TEST(CustomFormatString, DebugLevel) {
PW_LOG_DEBUG("This log statement should be at DEBUG level; no args");
Expand Down
72 changes: 51 additions & 21 deletions pw_log/basic_log_test_plain_c.c
Original file line number Diff line number Diff line change
Expand Up @@ -67,29 +67,54 @@ void BasicLogTestPlainC(void) {
PW_LOG_CRITICAL("This log is the last one; device should reboot");

// Core log macro, with manually specified level and flags.
PW_LOG(PW_LOG_LEVEL_DEBUG, 0, "A manual DEBUG-level message");
PW_LOG(PW_LOG_LEVEL_DEBUG, 1, "A manual DEBUG-level message; with a flag");
PW_LOG(PW_LOG_LEVEL_DEBUG,
PW_LOG_MODULE_NAME,
0,
"A manual DEBUG-level message");
PW_LOG(PW_LOG_LEVEL_DEBUG,
PW_LOG_MODULE_NAME,
1,
"A manual DEBUG-level message; with a flag");

PW_LOG(PW_LOG_LEVEL_INFO, 0, "A manual INFO-level message");
PW_LOG(PW_LOG_LEVEL_INFO, 1, "A manual INFO-level message; with a flag");

PW_LOG(PW_LOG_LEVEL_WARN, 0, "A manual WARN-level message");
PW_LOG(PW_LOG_LEVEL_WARN, 1, "A manual WARN-level message; with a flag");

PW_LOG(PW_LOG_LEVEL_ERROR, 0, "A manual ERROR-level message");
PW_LOG(PW_LOG_LEVEL_ERROR, 1, "A manual ERROR-level message; with a flag");
PW_LOG(
PW_LOG_LEVEL_INFO, PW_LOG_MODULE_NAME, 0, "A manual INFO-level message");
PW_LOG(PW_LOG_LEVEL_INFO,
PW_LOG_MODULE_NAME,
1,
"A manual INFO-level message; with a flag");

PW_LOG(PW_LOG_LEVEL_CRITICAL, 0, "A manual CRITICAL-level message");
PW_LOG(
PW_LOG_LEVEL_CRITICAL, 1, "A manual CRITICAL-level message; with a flag");
PW_LOG_LEVEL_WARN, PW_LOG_MODULE_NAME, 0, "A manual WARN-level message");
PW_LOG(PW_LOG_LEVEL_WARN,
PW_LOG_MODULE_NAME,
1,
"A manual WARN-level message; with a flag");

PW_LOG(PW_LOG_LEVEL_ERROR,
PW_LOG_MODULE_NAME,
0,
"A manual ERROR-level message");
PW_LOG(PW_LOG_LEVEL_ERROR,
PW_LOG_MODULE_NAME,
1,
"A manual ERROR-level message; with a flag");

PW_LOG(PW_LOG_LEVEL_CRITICAL,
PW_LOG_MODULE_NAME,
0,
"A manual CRITICAL-level message");
PW_LOG(PW_LOG_LEVEL_CRITICAL,
PW_LOG_MODULE_NAME,
1,
"A manual CRITICAL-level message; with a flag");

// Log levels other than the standard ones work; what each backend does is
// implementation defined.
PW_LOG(0, PW_LOG_FLAGS, "Custom log level: 0");
PW_LOG(1, PW_LOG_FLAGS, "Custom log level: 1");
PW_LOG(2, PW_LOG_FLAGS, "Custom log level: 2");
PW_LOG(3, PW_LOG_FLAGS, "Custom log level: 3");
PW_LOG(100, PW_LOG_FLAGS, "Custom log level: 100");
PW_LOG(0, PW_LOG_MODULE_NAME, PW_LOG_FLAGS, "Custom log level: 0");
PW_LOG(1, PW_LOG_MODULE_NAME, PW_LOG_FLAGS, "Custom log level: 1");
PW_LOG(2, PW_LOG_MODULE_NAME, PW_LOG_FLAGS, "Custom log level: 2");
PW_LOG(3, PW_LOG_MODULE_NAME, PW_LOG_FLAGS, "Custom log level: 3");
PW_LOG(100, PW_LOG_MODULE_NAME, PW_LOG_FLAGS, "Custom log level: 100");

// Logging from a function.
LoggingFromFunctionPlainC();
Expand All @@ -104,15 +129,20 @@ void BasicLogTestPlainC(void) {
}

#undef PW_LOG
#define PW_LOG(level, flags, message, ...) \
DoNothingFakeFunction("%d/%d/%d: incoming transmission [" message "]", \
#define PW_LOG(level, module, flags, message, ...) \
DoNothingFakeFunction(module, \
"%d/%d/%d: incoming transmission [" message "]", \
level, \
__LINE__, \
flags PW_COMMA_ARGS(__VA_ARGS__))

static void DoNothingFakeFunction(const char* f, ...) PW_PRINTF_FORMAT(1, 2);
static void DoNothingFakeFunction(const char* module, const char* f, ...)
PW_PRINTF_FORMAT(2, 3);

static void DoNothingFakeFunction(const char* f, ...) { (void)f; }
static void DoNothingFakeFunction(const char* module, const char* f, ...) {
(void)module;
(void)f;
}

static void CustomFormatStringTest(void) {
PW_LOG_DEBUG("Abc");
Expand Down
21 changes: 12 additions & 9 deletions pw_log/docs.rst
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,15 @@ Logging macros
These are the primary macros for logging information about the functioning of a
system, intended to be used directly.

.. cpp:function:: PW_LOG(level, flags, fmt, ...)
.. c:macro:: PW_LOG(level, module, flags, fmt, ...)
This is the primary mechanism for logging.

*level* - An integer level as defined by ``pw_log/levels.h``.

*module* - A string literal for the module name. Defaults to
:c:macro:`PW_LOG_MODULE_NAME`.

*flags* - Arbitrary flags the backend can leverage. The semantics of these
flags are not defined in the facade, but are instead meant as a general
mechanism for communication bits of information to the logging backend.
Expand All @@ -104,8 +107,8 @@ system, intended to be used directly.

.. code-block:: cpp
PW_LOG(PW_LOG_FLAGS, PW_LOG_LEVEL_INFO, "Temp is %d degrees", temp);
PW_LOG(UNRELIABLE_DELIVERY, PW_LOG_LEVEL_ERROR, "It didn't work!");
PW_LOG(PW_LOG_LEVEL_INFO, PW_LOG_MODULE_NAME, PW_LOG_FLAGS, "Temp is %d degrees", temp);
PW_LOG(PW_LOG_LEVEL_ERROR, PW_LOG_MODULE_NAME, UNRELIABLE_DELIVERY, "It didn't work!");
.. note::

Expand All @@ -116,13 +119,13 @@ system, intended to be used directly.
in the backend.


.. cpp:function:: PW_LOG_DEBUG(fmt, ...)
.. cpp:function:: PW_LOG_INFO(fmt, ...)
.. cpp:function:: PW_LOG_WARN(fmt, ...)
.. cpp:function:: PW_LOG_ERROR(fmt, ...)
.. cpp:function:: PW_LOG_CRITICAL(fmt, ...)
.. c:macro:: PW_LOG_DEBUG(fmt, ...)
.. c:macro:: PW_LOG_INFO(fmt, ...)
.. c:macro:: PW_LOG_WARN(fmt, ...)
.. c:macro:: PW_LOG_ERROR(fmt, ...)
.. c:macro:: PW_LOG_CRITICAL(fmt, ...)
Shorthand for `PW_LOG(PW_LOG_FLAGS, <level>, fmt, ...)`.
Shorthand for ``PW_LOG(<level>, PW_LOG_MODULE_NAME, PW_LOG_FLAGS, fmt, ...)``.

--------------------
Module configuration
Expand Down
2 changes: 1 addition & 1 deletion pw_log/public/pw_log/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,5 +37,5 @@
// This expression determines whether or not the statement is enabled and
// should be passed to the backend.
#ifndef PW_LOG_ENABLE_IF_DEFAULT
#define PW_LOG_ENABLE_IF_DEFAULT(level, flags) ((level) >= PW_LOG_LEVEL)
#define PW_LOG_ENABLE_IF_DEFAULT(level, module, flags) ((level) >= PW_LOG_LEVEL)
#endif // PW_LOG_ENABLE_IF_DEFAULT
17 changes: 10 additions & 7 deletions pw_log/public/pw_log/internal/glog_adapter.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,16 @@ class GlogStreamingLog {

// Declares a unique GlogStreamingLog class definition with a destructor which
// matches the desired pw_log_level.
#define _PW_LOG_GLOG_DECLARATION_PW_LOG(pw_log_level, unique) \
class unique : public ::pw::log::internal::GlogStreamingLog { \
public: \
~unique() { \
PW_HANDLE_LOG( \
pw_log_level, PW_LOG_FLAGS, "%s", string_builder_.c_str()); \
} \
#define _PW_LOG_GLOG_DECLARATION_PW_LOG(pw_log_level, unique) \
class unique : public ::pw::log::internal::GlogStreamingLog { \
public: \
~unique() { \
PW_HANDLE_LOG(pw_log_level, \
PW_LOG_MODULE_NAME, \
PW_LOG_FLAGS, \
"%s", \
string_builder_.c_str()); \
} \
}

// Declares a unique GlogStreamingLog class definition with a destructor which
Expand Down
Loading

0 comments on commit 4c95c0b

Please sign in to comment.