[PATCH 07/14] log: Avoid including function names by default
Sean Anderson
seanga2 at gmail.com
Sat Jul 20 17:52:33 CEST 2024
On 7/20/24 02:16, Simon Glass wrote:
> Unless function names are requested, the logging system should not
> compile these into the code. Adjust the macros to handle this.
>
> Enable CONFIG_LOGF_FUNC logging for sandbox since the tests expect the
> function names to be included. Fix up the pinmux test which checks a
> logging statement.
>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>
> common/log_console.c | 4 ++--
> configs/sandbox_defconfig | 1 +
> include/log.h | 18 ++++++++++++------
> test/cmd/pinmux.c | 8 +++++++-
> test/log/log_test.c | 4 ++--
> 5 files changed, 24 insertions(+), 11 deletions(-)
>
> diff --git a/common/log_console.c b/common/log_console.c
> index c27101b8fe2..9376baad664 100644
> --- a/common/log_console.c
> +++ b/common/log_console.c
> @@ -38,10 +38,10 @@ static int log_console_emit(struct log_device *ldev, struct log_rec *rec)
> printf("%d-", rec->line);
> if (fmt & BIT(LOGF_FUNC)) {
> if (CONFIG_IS_ENABLED(USE_TINY_PRINTF)) {
> - printf("%s()", rec->func);
> + printf("%s()", rec->func ?: "?");
> } else {
> printf("%*s()", CONFIG_LOGF_FUNC_PAD,
> - rec->func);
> + rec->func ?: "?");
> }
> }
> }
> diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> index e2db66d4a25..d43d92d0cf2 100644
> --- a/configs/sandbox_defconfig
> +++ b/configs/sandbox_defconfig
> @@ -38,6 +38,7 @@ CONFIG_PRE_CONSOLE_BUFFER=y
> CONFIG_LOG=y
> CONFIG_LOG_MAX_LEVEL=9
> CONFIG_LOG_DEFAULT_LEVEL=6
> +CONFIG_LOGF_FUNC=y
> CONFIG_DISPLAY_BOARDINFO_LATE=y
> CONFIG_STACKPROTECTOR=y
> CONFIG_ANDROID_AB=y
> diff --git a/include/log.h b/include/log.h
> index fc0d5984472..67b6fd70050 100644
> --- a/include/log.h
> +++ b/include/log.h
> @@ -125,7 +125,7 @@ static inline int log_uc_cat(enum uclass_id id)
> * @level: Level of log record (indicating its severity)
> * @file: File name of file where log record was generated
> * @line: Line number in file where log record was generated
> - * @func: Function where log record was generated
> + * @func: Function where log record was generated, NULL if not known
> * @fmt: printf() format string for log record
> * @...: Optional parameters, according to the format string @fmt
> * Return: 0 if log record was emitted, -ve on error
> @@ -141,7 +141,7 @@ int _log(enum log_category_t cat, enum log_level_t level, const char *file,
> * @level: Level of log record (indicating its severity)
> * @file: File name of file where log record was generated
> * @line: Line number in file where log record was generated
> - * @func: Function where log record was generated
> + * @func: Function where log record was generated, NULL if not known
> * @addr: Starting address to display at start of line
> * @data: pointer to data buffer
> * @width: data value width. May be 1, 2, or 4.
> @@ -193,6 +193,12 @@ int _log_buffer(enum log_category_t cat, enum log_level_t level,
> #define _LOG_DEBUG 0
> #endif
>
> +#ifdef CONFIG_LOGF_FUNC
> +#define _log_func __func__
> +#else
> +#define _log_func NULL
> +#endif
> +
> #if CONFIG_IS_ENABLED(LOG)
>
> /* Emit a log record if the level is less that the maximum */
> @@ -201,7 +207,7 @@ int _log_buffer(enum log_category_t cat, enum log_level_t 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__, \
> + __LINE__, _log_func, \
> pr_fmt(_fmt), ##_args); \
> })
>
> @@ -211,7 +217,7 @@ int _log_buffer(enum log_category_t cat, enum log_level_t level,
> if (_LOG_DEBUG != 0 || _l <= _LOG_MAX_LEVEL) \
> _log_buffer((enum log_category_t)(_cat), \
> (enum log_level_t)(_l | _LOG_DEBUG), __FILE__, \
> - __LINE__, __func__, _addr, _data, \
> + __LINE__, _log_func, _addr, _data, \
> _width, _count, _linelen); \
> })
> #else
> @@ -302,7 +308,7 @@ void __assert_fail(const char *assertion, const char *file, unsigned int line,
> */
> #define assert(x) \
> ({ if (!(x) && _DEBUG) \
> - __assert_fail(#x, __FILE__, __LINE__, __func__); })
> + __assert_fail(#x, __FILE__, __LINE__, _log_func); })
Can we keep this one in? It's only compiled for debug and we are including file/line anyway.
> /*
> * This one actually gets compiled in even without DEBUG. It doesn't include the
> @@ -314,7 +320,7 @@ void __assert_fail(const char *assertion, const char *file, unsigned int line,
> #define assert_noisy(x) \
> ({ bool _val = (x); \
> if (!_val) \
> - __assert_fail(#x, "?", __LINE__, __func__); \
> + __assert_fail(#x, "?", __LINE__, _log_func); \
> _val; \
> })
>
> diff --git a/test/cmd/pinmux.c b/test/cmd/pinmux.c
> index 4253baa5646..7ab7004b684 100644
> --- a/test/cmd/pinmux.c
> +++ b/test/cmd/pinmux.c
> @@ -30,7 +30,13 @@ static int dm_test_cmd_pinmux_status_pinname(struct unit_test_state *uts)
>
> console_record_reset();
> run_command("pinmux status P9", 0);
> - ut_assert_nextlinen("single-pinctrl pinctrl-single-no-width: missing register width");
> + if (IS_ENABLED(CONFIG_LOGF_FUNC)) {
> + ut_assert_nextlinen(
> + " single_of_to_plat() single-pinctrl pinctrl-single-no-width: missing register width");
> + } else {
> + ut_assert_nextlinen(
> + "single-pinctrl pinctrl-single-no-width: missing register width");
> + }
> ut_assert_nextlinen("P9 not found");
> ut_assert_console_end();
>
> diff --git a/test/log/log_test.c b/test/log/log_test.c
> index 855353a9c40..d756b96dbac 100644
> --- a/test/log/log_test.c
> +++ b/test/log/log_test.c
> @@ -452,8 +452,8 @@ int log_test_buffer(struct unit_test_state *uts)
> /* This one should product no output due to the debug level */
> log_buffer(LOGC_BOOT, LOGL_DEBUG, 0, buf, 1, 0x12, 0);
>
> - ut_assert_nextline("00000000: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff ..\"3DUfw........");
> - ut_assert_nextline("00000010: 10 00 ..");
> + ut_assert_nextline(" log_test_buffer() 00000000: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff ..\"3DUfw........");
> + ut_assert_nextline(" log_test_buffer() 00000010: 10 00 ..");
> ut_assert_console_end();
> free(buf);
>
Anyway, the rest looks fine. This probably breaks the use case of someone turning on
function names at runtime, so maybe we will need to split this config if someone cares.
--Sean
More information about the U-Boot
mailing list