refactor nfs_errorf() and nfs_ferrorf() to the standard do-while(0)
pattern for safer macro expansion and kernel style compliance.
additionally, remove nfs_warnf() and nfs_fwarnf() as git grep
confirms they have no callers in the current tree.
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202603110038.P6d14oxa-lkp@intel.com/
Signed-off-by: Sean Chang <seanwascoding@gmail.com>
---
fs/nfs/internal.h | 28 +++++++++++++---------------
1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 63e09dfc27a8..59ab43542390 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -161,13 +161,19 @@ enum nfs_lock_status {
NFS_LOCK_NOLOCK = 2,
};
-#define nfs_errorf(fc, fmt, ...) ((fc)->log.log ? \
- errorf(fc, fmt, ## __VA_ARGS__) : \
- ({ dprintk(fmt "\n", ## __VA_ARGS__); }))
-
-#define nfs_ferrorf(fc, fac, fmt, ...) ((fc)->log.log ? \
- errorf(fc, fmt, ## __VA_ARGS__) : \
- ({ dfprintk(fac, fmt "\n", ## __VA_ARGS__); }))
+#define nfs_errorf(fc, fmt, ...) do { \
+ if ((fc)->log.log) \
+ errorf(fc, fmt, ## __VA_ARGS__); \
+ else \
+ dprintk(fmt "\n", ## __VA_ARGS__); \
+} while (0)
+
+#define nfs_ferrorf(fc, fac, fmt, ...) do { \
+ if ((fc)->log.log) \
+ errorf(fc, fmt, ## __VA_ARGS__); \
+ else \
+ dfprintk(fac, fmt "\n", ## __VA_ARGS__); \
+} while (0)
#define nfs_invalf(fc, fmt, ...) ((fc)->log.log ? \
invalf(fc, fmt, ## __VA_ARGS__) : \
@@ -177,14 +183,6 @@ enum nfs_lock_status {
invalf(fc, fmt, ## __VA_ARGS__) : \
({ dfprintk(fac, fmt "\n", ## __VA_ARGS__); -EINVAL; }))
-#define nfs_warnf(fc, fmt, ...) ((fc)->log.log ? \
- warnf(fc, fmt, ## __VA_ARGS__) : \
- ({ dprintk(fmt "\n", ## __VA_ARGS__); }))
-
-#define nfs_fwarnf(fc, fac, fmt, ...) ((fc)->log.log ? \
- warnf(fc, fmt, ## __VA_ARGS__) : \
- ({ dfprintk(fac, fmt "\n", ## __VA_ARGS__); }))
-
static inline struct nfs_fs_context *nfs_fc2context(const struct fs_context *fc)
{
return fc->fs_private;
--
2.34.1On Thu, Mar 19, 2026 at 10:18:46PM +0800, Sean Chang wrote:
> refactor nfs_errorf() and nfs_ferrorf() to the standard do-while(0)
> pattern for safer macro expansion and kernel style compliance.
>
> additionally, remove nfs_warnf() and nfs_fwarnf() as git grep
> confirms they have no callers in the current tree.
...
> #define nfs_invalf(fc, fmt, ...) ((fc)->log.log ? \
> invalf(fc, fmt, ## __VA_ARGS__) : \
> invalf(fc, fmt, ## __VA_ARGS__) : \
> ({ dfprintk(fac, fmt "\n", ## __VA_ARGS__); -EINVAL; }))
Why not all of them?
--
With Best Regards,
Andy Shevchenko
On Thu, Mar 19, 2026 at 10:41 PM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Thu, Mar 19, 2026 at 10:18:46PM +0800, Sean Chang wrote:
> > refactor nfs_errorf() and nfs_ferrorf() to the standard do-while(0)
> > pattern for safer macro expansion and kernel style compliance.
> >
> > additionally, remove nfs_warnf() and nfs_fwarnf() as git grep
> > confirms they have no callers in the current tree.
>
> ...
>
> > #define nfs_invalf(fc, fmt, ...) ((fc)->log.log ? \
> > invalf(fc, fmt, ## __VA_ARGS__) : \
>
> > invalf(fc, fmt, ## __VA_ARGS__) : \
> > ({ dfprintk(fac, fmt "\n", ## __VA_ARGS__); -EINVAL; }))
>
> Why not all of them?
>
I initially only refactored nfs_errorf because it doesn't return a value.
For nfs_invalf, it will always return -EINVAL. Would you prefer me to
refactor it using the ({ ... }) statement expression pattern to keep the
return value, or is it better to leave it as is ?
#define nfs_invalf(fc, fmt, ...) ({ \
if ((fc)->log.log) \
invalf(fc, fmt, ## __VA_ARGS__); \
else \
dfprintk(fac, fmt "\n", ## __VA_ARGS__);\
-EINVAL; \
})
On Thu, Mar 19, 2026 at 10:59:02PM +0800, Sean Chang wrote:
> On Thu, Mar 19, 2026 at 10:41 PM Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> > On Thu, Mar 19, 2026 at 10:18:46PM +0800, Sean Chang wrote:
...
> > > #define nfs_invalf(fc, fmt, ...) ((fc)->log.log ? \
> > > invalf(fc, fmt, ## __VA_ARGS__) : \
> >
> > > invalf(fc, fmt, ## __VA_ARGS__) : \
> > > ({ dfprintk(fac, fmt "\n", ## __VA_ARGS__); -EINVAL; }))
> >
> > Why not all of them?
>
> I initially only refactored nfs_errorf because it doesn't return a value.
> For nfs_invalf, it will always return -EINVAL. Would you prefer me to
> refactor it using the ({ ... }) statement expression pattern to keep the
> return value, or is it better to leave it as is ?
I don't think in this case it improves the situation. Yeah, it's unfortunate.
> #define nfs_invalf(fc, fmt, ...) ({ \
> if ((fc)->log.log) \
> invalf(fc, fmt, ## __VA_ARGS__); \
I believe this already has an error code inside, that's why it's only added to
the 'else' branch.
> else \
> dfprintk(fac, fmt "\n", ## __VA_ARGS__);\
> -EINVAL; \
> })
Okay, let's go with your original approach (ideally these all probably should
be replaced by static inline:s).
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Tested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Unrelated to the series, but if you want to address these:
nfs/super.c:1170:49: warning: incorrect type in initializer (different address spaces)
nfs/super.c:1170:49: expected struct rpc_xprt *xprt1
nfs/super.c:1170:49: got struct rpc_xprt [noderef] __rcu *cl_xprt
nfs/super.c:1171:49: warning: incorrect type in initializer (different address spaces)
nfs/super.c:1171:49: expected struct rpc_xprt *xprt2
nfs/super.c:1171:49: got struct rpc_xprt [noderef] __rcu *cl_xprt
nfs/./nfstrace.h:1488:1: warning: dereference of noderef expression
nfs/./nfs4trace.h:2168:1: error: too long token expansion
nfs/./nfs4trace.h:2234:1: error: too long token expansion
--
With Best Regards,
Andy Shevchenko
On Thu, Mar 19, 2026 at 11:49 PM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> > > > #define nfs_invalf(fc, fmt, ...) ((fc)->log.log ? \
> > > > invalf(fc, fmt, ## __VA_ARGS__) : \
> > >
> > > > invalf(fc, fmt, ## __VA_ARGS__) : \
> > > > ({ dfprintk(fac, fmt "\n", ## __VA_ARGS__); -EINVAL; }))
> > >
> > > Why not all of them?
> >
> > I initially only refactored nfs_errorf because it doesn't return a value.
> > For nfs_invalf, it will always return -EINVAL. Would you prefer me to
> > refactor it using the ({ ... }) statement expression pattern to keep the
> > return value, or is it better to leave it as is ?
>
> I don't think in this case it improves the situation. Yeah, it's unfortunate.
>
> > #define nfs_invalf(fc, fmt, ...) ({ \
> > if ((fc)->log.log) \
> > invalf(fc, fmt, ## __VA_ARGS__); \
>
> I believe this already has an error code inside, that's why it's only added to
> the 'else' branch.
>
> > else \
> > dfprintk(fac, fmt "\n", ## __VA_ARGS__);\
> > -EINVAL; \
> > })
>
> Okay, let's go with your original approach (ideally these all probably should
> be replaced by static inline:s).
>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> Tested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
>
Thanks for your review and for providing the tags!
I'll include your Reviewed-by and Tested-by tags in the v4 submission.
> Unrelated to the series, but if you want to address these:
>
> nfs/super.c:1170:49: warning: incorrect type in initializer (different address spaces)
> nfs/super.c:1170:49: expected struct rpc_xprt *xprt1
> nfs/super.c:1170:49: got struct rpc_xprt [noderef] __rcu *cl_xprt
> nfs/super.c:1171:49: warning: incorrect type in initializer (different address spaces)
> nfs/super.c:1171:49: expected struct rpc_xprt *xprt2
> nfs/super.c:1171:49: got struct rpc_xprt [noderef] __rcu *cl_xprt
>
> nfs/./nfstrace.h:1488:1: warning: dereference of noderef expression
> nfs/./nfs4trace.h:2168:1: error: too long token expansion
> nfs/./nfs4trace.h:2234:1: error: too long token expansion
>
Regarding the Sparse warnings in super.c and the trace header errors,
I've noted them down. I'll look into them separately and may submit a
follow-up patch to address them later.
Best Regards,
Sean
© 2016 - 2026 Red Hat, Inc.