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

[Quentin Schulz <quentin.schulz@cherry.de>]

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

I don't really see how that is better than simply calling

return log_msg_retz("abc", my_function());

?

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?

Cheers,
Quentin