[PATCH v2 6/6] log: Convert log values to printf() if not enabled
Sean Anderson
seanga2 at gmail.com
Fri Feb 5 02:28:28 CET 2021
On 1/20/21 10:10 PM, Simon Glass wrote:
> At present if logging not enabled, log_info() becomes a nop. But we want
> log output at the 'info' level to be akin to printf(). Update the macro to
> pass the output straight to printf() in this case.
>
> This mimics the behaviour for the log_...() macros like log_debug() and
> log_info(), so we can drop the special case for these.
>
> Add new tests to cover this case.
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>
> Changes in v2:
> - Update commit message and cover letter to mention log_...() macros
> - Add a test for !CONFIG_LOG
> - Update log() to (effectively) call debug() for log_level == LOGL_DEBUG
>
> doc/develop/logging.rst | 6 ++++--
> include/log.h | 33 ++++++++++++++-------------------
> test/log/Makefile | 1 +
> test/log/nolog_ndebug.c | 38 ++++++++++++++++++++++++++++++++++++++
> test/log/nolog_test.c | 3 +++
> 5 files changed, 60 insertions(+), 21 deletions(-)
> create mode 100644 test/log/nolog_ndebug.c
>
> diff --git a/doc/develop/logging.rst b/doc/develop/logging.rst
> index b6c6b45049f..55cef42664d 100644
> --- a/doc/develop/logging.rst
> +++ b/doc/develop/logging.rst
> @@ -54,6 +54,10 @@ If CONFIG_LOG is not set, then no logging will be available.
> The above have SPL and TPL versions also, e.g. CONFIG_SPL_LOG_MAX_LEVEL and
> CONFIG_TPL_LOG_MAX_LEVEL.
>
> +If logging is disabled, the default behaviour is to output any message at
> +level LOGL_INFO and below. If logging is disabled and DEBUG is defined (at
> +the very top of a C file) then any message at LOGL_DEBUG will be written.
> +
> Temporary logging within a single file
> --------------------------------------
>
> @@ -293,8 +297,6 @@ More logging destinations:
>
> Convert debug() statements in the code to log() statements
>
> -Support making printf() emit log statements at L_INFO level
> -
> Convert error() statements in the code to log() statements
>
> Figure out what to do with BUG(), BUG_ON() and warn_non_spl()
> diff --git a/include/log.h b/include/log.h
> index 6ef891d4d2d..748e34d5a26 100644
> --- a/include/log.h
> +++ b/include/log.h
> @@ -156,6 +156,10 @@ static inline int _log_nop(enum log_category_t cat, enum log_level_t level,
> */
> #if CONFIG_IS_ENABLED(LOG)
> #define _LOG_MAX_LEVEL CONFIG_VAL(LOG_MAX_LEVEL)
> +#else
> +#define _LOG_MAX_LEVEL LOGL_INFO
> +#endif
> +
> #define log_emer(_fmt...) log(LOG_CATEGORY, LOGL_EMERG, ##_fmt)
> #define log_alert(_fmt...) log(LOG_CATEGORY, LOGL_ALERT, ##_fmt)
> #define log_crit(_fmt...) log(LOG_CATEGORY, LOGL_CRIT, ##_fmt)
> @@ -167,41 +171,32 @@ static inline int _log_nop(enum log_category_t cat, enum log_level_t level,
> #define log_content(_fmt...) log(LOG_CATEGORY, LOGL_DEBUG_CONTENT, ##_fmt)
> #define log_io(_fmt...) log(LOG_CATEGORY, LOGL_DEBUG_IO, ##_fmt)
> #define log_cont(_fmt...) log(LOGC_CONT, LOGL_CONT, ##_fmt)
> -#else
> -#define _LOG_MAX_LEVEL LOGL_INFO
> -#define log_emerg(_fmt, ...) printf(_fmt, ##__VA_ARGS__)
> -#define log_alert(_fmt, ...) printf(_fmt, ##__VA_ARGS__)
> -#define log_crit(_fmt, ...) printf(_fmt, ##__VA_ARGS__)
> -#define log_err(_fmt, ...) printf(_fmt, ##__VA_ARGS__)
> -#define log_warning(_fmt, ...) printf(_fmt, ##__VA_ARGS__)
> -#define log_notice(_fmt, ...) printf(_fmt, ##__VA_ARGS__)
> -#define log_info(_fmt, ...) printf(_fmt, ##__VA_ARGS__)
> -#define log_cont(_fmt, ...) printf(_fmt, ##__VA_ARGS__)
> -#define log_debug(_fmt, ...) debug(_fmt, ##__VA_ARGS__)
> -#define log_content(_fmt...) log_nop(LOG_CATEGORY, \
> - LOGL_DEBUG_CONTENT, ##_fmt)
> -#define log_io(_fmt...) log_nop(LOG_CATEGORY, LOGL_DEBUG_IO, ##_fmt)
> -#endif
>
> -#if CONFIG_IS_ENABLED(LOG)
> #ifdef LOG_DEBUG
> #define _LOG_DEBUG LOGL_FORCE_DEBUG
> #else
> #define _LOG_DEBUG 0
> #endif
>
> +#if CONFIG_IS_ENABLED(LOG)
> +
> /* Emit a log record if the level is less that the maximum */
> #define log(_cat, _level, _fmt, _args...) ({ \
> int _l = _level; \
> - if (CONFIG_IS_ENABLED(LOG) && \
> - (_LOG_DEBUG != 0 || _l <= _LOG_MAX_LEVEL)) \
> + if (_LOG_DEBUG != 0 || _l <= _LOG_MAX_LEVEL) \
> _log((enum log_category_t)(_cat), \
> (enum log_level_t)(_l | _LOG_DEBUG), __FILE__, \
> __LINE__, __func__, \
> pr_fmt(_fmt), ##_args); \
> })
> #else
> -#define log(_cat, _level, _fmt, _args...)
> +/* Note: _LOG_DEBUG != 0 avoids a warning with clang */
> +#define log(_cat, _level, _fmt, _args...) ({ \
> + int _l = _level; \
> + if (_LOG_DEBUG != 0 || _l <= LOGL_INFO || \
Should this be < CONFIG_LOGLEVEL?
--Sean
> + (_DEBUG && _l == LOGL_DEBUG)) \
> + printf(_fmt, ##_args); \
> + })
> #endif
>
> #define log_nop(_cat, _level, _fmt, _args...) ({ \
> diff --git a/test/log/Makefile b/test/log/Makefile
> index 0e900363595..b63064e8d02 100644
> --- a/test/log/Makefile
> +++ b/test/log/Makefile
> @@ -19,6 +19,7 @@ obj-$(CONFIG_CONSOLE_RECORD) += cont_test.o
> obj-y += pr_cont_test.o
> else
> obj-$(CONFIG_CONSOLE_RECORD) += nolog_test.o
> +obj-$(CONFIG_CONSOLE_RECORD) += nolog_ndebug.o
> endif
>
> endif # CONFIG_UT_LOG
> diff --git a/test/log/nolog_ndebug.c b/test/log/nolog_ndebug.c
> new file mode 100644
> index 00000000000..5df353781d9
> --- /dev/null
> +++ b/test/log/nolog_ndebug.c
> @@ -0,0 +1,38 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2021 Google LLC
> + *
> + * Logging function tests for CONFIG_LOG=n without #define DEBUG
> + */
> +
> +#include <common.h>
> +#include <console.h>
> +#include <log.h>
> +#include <test/log.h>
> +#include <test/ut.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +#define BUFFSIZE 32
> +
> +static int log_test_log_disabled_ndebug(struct unit_test_state *uts)
> +{
> + char buf[BUFFSIZE];
> + int i;
> +
> + memset(buf, 0, BUFFSIZE);
> + console_record_reset_enable();
> +
> + /* Output a log record at every level */
> + for (i = LOGL_EMERG; i < LOGL_COUNT; i++)
> + log(LOGC_NONE, i, "testing level %i\n", i);
> + gd->flags &= ~GD_FLG_RECORD;
> +
> + /* Since DEBUG is not defined, we expect to not get debug output */
> + for (i = LOGL_EMERG; i < LOGL_DEBUG; i++)
> + ut_assertok(ut_check_console_line(uts, "testing level %d", i));
> + ut_assertok(ut_check_console_end(uts));
> +
> + return 0;
> +}
> +LOG_TEST(log_test_log_disabled_ndebug);
> diff --git a/test/log/nolog_test.c b/test/log/nolog_test.c
> index c418ed07c9a..dcf6b779bdc 100644
> --- a/test/log/nolog_test.c
> +++ b/test/log/nolog_test.c
> @@ -10,6 +10,7 @@
>
> #include <common.h>
> #include <console.h>
> +#include <log.h>
> #include <test/log.h>
> #include <test/test.h>
> #include <test/suites.h>
> @@ -127,8 +128,10 @@ static int log_test_nolog_debug(struct unit_test_state *uts)
> memset(buf, 0, BUFFSIZE);
> console_record_reset_enable();
> log_debug("testing %s\n", "log_debug");
> + log(LOGC_NONE, LOGL_DEBUG, "more %s\n", "log_debug");
> gd->flags &= ~GD_FLG_RECORD;
> ut_assertok(ut_check_console_line(uts, "testing log_debug"));
> + ut_assertok(ut_check_console_line(uts, "more log_debug"));
> ut_assertok(ut_check_console_end(uts));
> return 0;
> }
>
More information about the U-Boot
mailing list