From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sean Anderson Date: Thu, 4 Feb 2021 20:28:28 -0500 Subject: [PATCH v2 6/6] log: Convert log values to printf() if not enabled In-Reply-To: <20210121031057.559301-7-sjg@chromium.org> References: <20210121031057.559301-1-sjg@chromium.org> <20210121031057.559301-7-sjg@chromium.org> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > --- > > 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 > +#include > +#include > +#include > +#include > + > +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 > #include > +#include > #include > #include > #include > @@ -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; > } >