Re: [PATCH v2] sunrpc: simplify dprintk() macros and cleanup redundant debug guards

On Tue, Mar 17, 2026 at 10:20 PM Andy Shevchenko
[off-list ref] wrote:

quoted

quoted

quoted

quoted

Does this patch fixes also 202603110038.P6d14oxa-lkp@intel.com?

Regarding the LKP report:
I have reproduced the Sparse warning (void vs int mismatch) locally.
To resolve this, I'll use the do-while(0) form in v3 to ensure the macro
always evaluates to void:
-# define dfprintk(fac, ...)            no_printk(__VA_ARGS__)
-# define dfprintk_rcu(fac, ...)        no_printk(__VA_ARGS__)
+# define dfprintk(fac, ...)            do { no_printk(__VA_ARGS__); } while (0)
+# define dfprintk_rcu(fac, ...)        do { no_printk(__VA_ARGS__); } while (0)

Wouldn't be better to drop ({}) in nfs_error*() macros?

I understand that the ({}) wrapper might look redundant at first
glance. However,
even if I remove it, the return type remains an issue because no_printk() (which
dprintk() expands to) already contains its own ({}) statement expression.

To resolve this without refactoring errorf(), which hasn't been
touched in years,
I believe modifying dfprintk() is the better path.

One alternative is to explicitly force the return type to void like this:
({ dprintk(fmt "\n", ##__VA_ARGS__); (void)0; })

While this ensures the return type remains void (consistent with the
behavior before
dprintk() was redefined), it is admittedly clunky. We could wrap
(void)0 in a macro like
#define nothing_to_do ((void)0), but that adds unnecessary complexity.

Therefore, I still prefer Option 1, as it restores the original
behavior from before the
recent commits and maintains the cleanest code structure for this subsystem.
What do you think?

Personally I found the ({}) in nfs_error*() redundant and the point of those
functions not being touched in years doesn't work for internal APIs. Do you
know the reason why it wasn't touched? Perhaps there was nothing to do simply
with that until dprintk() issue reveals some (legacy?) stuff to improve.

I would not go with (void)0 approach, but I also think that dropping unneeded
(if confirmed) expression wrapping is a good thing to do.

Hi Andy,
Thanks for your comment, how about removing the ({}) and ternary operator to
prevent sparse throw incompatible types. And use the do{} while(0) to prevent
dangling-else problem.

#define nfs_errorf(fc, fmt, ...) do { \
    if ((fc)->log.log) \
        errorf(fc, fmt, ## __VA_ARGS__); \
    else \
        dprintk(fmt "\n", ## __VA_ARGS__); \
} while (0)