[PATCH] log: Add helpers for calling log_msg_ret() et al

[Simon Glass]

Hi Quentin,

On Wed, 19 Mar 2025 at 13:04, Quentin Schulz <quentin.schulz at cherry.de> wrote:
>
> Hi Simon,
>
> On 3/19/25 12:49 PM, Simon Glass wrote:
> > Logging of function return-values is used very frequently in U-Boot now.
> > Add a few helper macros to make it less verbose to use.
> >
> > It turns out that the log_ret() variants are not so useful, since it is
> > not obviously where the error is coming from. So only the log_msg_ret()
> > variants are worthy of these macros.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
> >
> >   include/log.h | 26 ++++++++++++++++++++++++++
> >   1 file changed, 26 insertions(+)
> >
> > diff --git a/include/log.h b/include/log.h
> > index 4f6d6a2c2cf..bdda7af570c 100644
> > --- a/include/log.h
> > +++ b/include/log.h
> > @@ -380,6 +380,32 @@ void __assert_fail(const char *assertion, const char *file, unsigned int line,
> >   #define log_msg_retz(_msg, _ret) ((void)(_msg), _ret)
> >   #endif
> >
> > +/*
> > + * LOGR() - helper macro for calling a function and logging error returns
> > + *
> > + * Logs if the function returns a negative value
> > + *
> > + * Usage:   LOGR("abc", my_function(...));
> > + */
> > +#define LOGR(_msg, _expr)    do {            \
> > +     int _ret = _expr;                       \
> > +     if (_ret < 0)                           \
> > +             return log_msg_ret(_msg, _ret); \
> > +     } while (0)
> > +
> > +/*
> > + * LOGZ() - helper macro for calling a function and logging error returns
> > + *
> > + * Logs if the function returns a non-zero value
> > + *
> > + * Usage:   LOGZ("abc", my_function(...));
> > + */
> > +#define LOGZ(_msg, _expr)    do {            \
> > +     int _ret = _expr;                       \
> > +     if (_ret)                               \
> > +             return log_msg_retz(_msg, _ret);        \
> > +     } while (0)
> > +
>
> Mmmm not sure this forced return call is a good idea, this would forbid
> us from performing some unwinding for example.

Yes, it is really only for simple cases. Without the return, there
isn't much value and you may as well not use this macro.

>
> I don't really see how that is better than simply calling
>
> return log_msg_retz("abc", my_function());

That's not the intention. It actually replaces:

ret = my_function();
if (ret)
    return_log_msg_ret("abc", ret);

I use this a lot in my code. You can't always just return, since there
may not be an error.

>
> ?
>
> If we were to keep this, I would recommend to rename the macro and fix
> the docstring because it does not only log if the function returns a
> non-zero value. It does actually return.
>
> So something like
>
> LOGZ_AND_RETURN(_msg, _expr)
>
> maybe?

Sure, longer names are easier to learn, but then it is so long I doubt
anyone would use it.

Perhaps LOG_RET() and LOG_RETZ() ? But they might get confused with
log_ret() and log_retz(), which I am actually thinking of deleting.

I would like the shortest possible name to avoid spilling functions
onto the next line all the time.

Regards,
SImon