[PATCH v2 6/6] log: Convert log values to printf() if not enabled

[Simon Glass]

Hi Sean,

On Thu, 4 Feb 2021 at 18:28, Sean Anderson <seanga2 at gmail.com> wrote:
>
> 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?

Well it could be, but in fact that value predates the log system and
I've been wondering how to remove it, to reconcile the pr_...() stuff
and the log subsystem.

CONFIG_LOGLEVEL should really go away in favour of CONFIG_LOG_MAX_LEVEL I think.

Regards,
Simon