1. Patchew
  2. linux
  3. [PATCH v5 0/5] sunrpc/nfs: cleanup redundant debug checks and refactor macros
  4. View patch

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

Sean Chang posted 5 patches 1 week, 6 days ago
[PATCH v5 1/5] sunrpc: Fix dprintk type mismatch using do-while(0)
Posted by Sean Chang 1 week, 6 days ago 
Following David Laight's suggestion, simplify the macro definitions by removing
the unnecessary 'fmt' argument and using no_printk(VA_ARGS) directly.

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.

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
 
 /*
-- 
2.34.1
Re: [PATCH v5 1/5] sunrpc: Fix dprintk type mismatch using do-while(0)
Posted by Chuck Lever 1 week, 6 days ago 
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
Re: [PATCH v5 1/5] sunrpc: Fix dprintk type mismatch using do-while(0)
Posted by Sean Chang 1 week, 2 days ago 
On Sun, Mar 22, 2026 at 12:38 AM Chuck Lever <chuck.lever@oracle.com> wrote:
>
> 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?
>

Hi Chuck,
I notice that the commit messages are wrong because the current situation
is not the same as when I first sent it, so I will fix this message to the
correct one.

It may be like:
When RPC debugging is enabled, the dfprintk macros include redundant
else-branches that call no_printk(). Since no_printk() is a no-op
designed specifically for compile-time type checking, it is unnecessary
to invoke it explicitly when the ifdebug(fac) condition is not met
within a debug-enabled build.

Drop the explicit 'fmt' argument to allow the compiler to handle all
arguments directly through __VA_ARGS__. Since no_printk()
internally performs the same format string validation, passing the
entire argument list is more efficient and reduces macro complexity.

Best Regards,
Sean

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.