[PATCH v2 07/14] log: Avoid including function names by default

[Simon Glass]

Hi Quentin,

On Tue, 6 Aug 2024 at 08:16, Quentin Schulz <quentin.schulz at 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 <sjg at 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