Hi Quentin,
On Tue, 6 Aug 2024 at 08:16, Quentin Schulz <quentin.sch...@cherry.de> wrote:
>
> Hi Simon,
>
> On 7/21/24 5:25 PM, Simon Glass wrote:
> > Unless function names are requested, the logging system should not
> > compile these into the code. Adjust the macros to handle this.
> >
> > This means that turning on function names at runtime won't work unless
> > CONFIG_LOGF_FUNC is enabled. We could perhaps split this into a
> > separate option if that is a problem.
> >
> > 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.
> >
>
> I now understand the statement in patches earlier in this series :)
>
> > Signed-off-by: Simon Glass <s...@chromium.org>
> > ---
> >
> > Changes in v2:
> > - Update commit message to mention the runtime impact
> > - Leave assert() alone since it is only compiled in with LOG_DEBUG
> >
> > common/log_console.c | 4 ++--
> > configs/sandbox_defconfig | 1 +
> > include/log.h | 16 +++++++++++-----
> > test/cmd/pinmux.c | 8 +++++++-
> > test/log/log_test.c | 4 ++--
> > 5 files changed, 23 insertions(+), 10 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 ?: "?");
>
> What about setting _log_func to "?" if LOGF_FUNC isn't set?
That would require all call sites to pass a pointer to the string,
rather than just NULL. I suspect it would increase code size?
>
> > } else {
> > printf("%*s()", CONFIG_LOGF_FUNC_PAD,
> > - rec->func);
> > + rec->func ?: "?");
>
> I think you missed a similar change in common/log_syslog.c if I can
> trust my grep-fu?
Yes, thanks.
Regards,
Simon
