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 <s...@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
