Re: [PATCH v5 1/5] sunrpc: Fix dprintk type mismatch using do-while(0)

[Chuck Lever <chuck.lever@oracle.com>]

On 3/21/26 10:15 AM, Sean Chang wrote:
> Following David Laight's suggestion, simplify the macro definitions by removing
> the unnecessary 'fmt' argument and using no_printk(VA_ARGS) directly.

Generally we prefer a commit message to open with an explanation of why
the change is needed. Your first paragraph instead opens with what was
done ("Following David Laight's suggestion, simplify ...") rather than
why the change is necessary. The Sparse warning motivation is buried in
the second paragraph. Consider leading with a problem statement in this
and subsequent patches in this series.


> To resolve a Sparse warning (void vs int mismatch) when dfprintk is used in
> conditional statements, wrap the non-debug definition in a do-while(0) block.
> This ensures the macro always evaluates to a void expression.

The non-debug definitions in the diff below are:

> -# define dfprintk(fac, fmt, ...)        no_printk(fmt, ##__VA_ARGS__)
> -# define dfprintk_rcu(fac, fmt, ...)    no_printk(fmt, ##__VA_ARGS__)
> +# define dfprintk(fac, ...)             no_printk(__VA_ARGS__)
> +# define dfprintk_rcu(fac, ...) no_printk(__VA_ARGS__)

These are not wrapped in do { ... } while (0), and no_printk()
evaluates to int (0), not void. The do-while(0) wrapping that
was discussed on the list and fixes the Sparse warning appears
to be in a later patch in this series (the nfs_errorf
refactoring), not in this one.

Should the commit message second paragraph be removed or revised
to reflect what this patch actually does?


> Suggested-by: David Laight <david.laight.linux@gmail.com>
> Signed-off-by: Sean Chang <seanwascoding@gmail.com>
> ---
>  include/linux/sunrpc/debug.h | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/sunrpc/debug.h b/include/linux/sunrpc/debug.h
> index ab61bed2f7af..f6f2a106eeaf 100644
> --- a/include/linux/sunrpc/debug.h
> +++ b/include/linux/sunrpc/debug.h
> @@ -38,8 +38,6 @@ extern unsigned int		nlm_debug;
>  do {									\
>  	ifdebug(fac)							\
>  		__sunrpc_printk(fmt, ##__VA_ARGS__);			\
> -	else								\
> -		no_printk(fmt, ##__VA_ARGS__);				\
>  } while (0)
>  
>  # define dfprintk_rcu(fac, fmt, ...)					\
> @@ -48,15 +46,13 @@ do {									\
>  		rcu_read_lock();					\
>  		__sunrpc_printk(fmt, ##__VA_ARGS__);			\
>  		rcu_read_unlock();					\
> -	} else {							\
> -		no_printk(fmt, ##__VA_ARGS__);				\
>  	}								\
>  } while (0)
>  
>  #else
>  # define ifdebug(fac)		if (0)
> -# define dfprintk(fac, fmt, ...)	no_printk(fmt, ##__VA_ARGS__)
> -# define dfprintk_rcu(fac, fmt, ...)	no_printk(fmt, ##__VA_ARGS__)
> +# define dfprintk(fac, ...)		no_printk(__VA_ARGS__)
> +# define dfprintk_rcu(fac, ...)	no_printk(__VA_ARGS__)
>  #endif
>  
>  /*


-- 
Chuck Lever