Re: [PATCH v4 1/2] sunrpc: fix unused variable warnings by using no_printk

[Sean Chang]

On Sat, Feb 28, 2026 at 10:56 PM Sean Chang <seanwascoding@xxxxxxxxx> wrote:
>
> On Sat, Feb 28, 2026 at 2:15 AM David Laight
> <david.laight.linux@xxxxxxxxx> wrote:
> >
> > On Fri, 27 Feb 2026 18:57:33 +0100
> > Andrew Lunn <andrew@xxxxxxx> wrote:
> >
> > > > >  # define ifdebug(fac)            if (0)
> > > > > -# define dfprintk(fac, fmt, ...) do {} while (0)
> > > > > -# define dfprintk_rcu(fac, fmt, ...)     do {} while (0)
> > > > > +# define dfprintk(fac, fmt, ...) no_printk(fmt, ##__VA_ARGS__)
> > > > > +# define dfprintk_rcu(fac, fmt, ...)     no_printk(fmt, ##__VA_ARGS__)
> > > >
> > > > You can omit fmt, then you don't need the ##
> > > > #define dfprintk(fac, ...)  no_printk(__VA_ARGS__)
> > >
> > > /*
> > >  * Dummy printk for disabled debugging statements to use whilst maintaining
> > >  * gcc's format checking.
> > >  */
> > > #define no_printk(fmt, ...)                           \
> > > ({                                                    \
> > >       if (0)                                          \
> > >               _printk(fmt, ##__VA_ARGS__);            \
> > >       0;                                              \
> > > })
> > >
> > > Without fmt, gcc cannot do format checking. Or worse, it takes the
> > > first member of __VA_ARGS__ as the format, and gives spurious errors?
> >
> > By the time the compiler looks at it the pre-processor has expanded
> > __VA_ARGS__.
> > So it doesn't matter that the format string is in the __VA_ARGS__
> > list rather than preceding it.
> >
>
> Hi David,
> Thanks for the explanation. I agree that since __VA_ARGS__ will
> always contain the format string in this context, the ## and explicit
> fmt are indeed redundant. Since no_printk itself already handles
> the ##__VA_ARGS__ logic internally, there's no need to duplicate
> it in the dfprintk definition.
>
> I'll switch to the simpler in v5:
> -# 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__)
>

HI all,
I've also identified the cause of the build error in fs/nfsd/nfsfh.c
reported by syzbot [1]. The "use of undeclared identifier 'buf'" occurs
because buf is only declared within the RPC_IFDEBUG macro,
which is removed when CONFIG_SUNRPC_DEBUG is disabled.
To fix this, I will follow the pattern used in net/sunrpc/xprtrdma/
svc_rdma_transport.c by wrapping the dprintk call that references
buf within an #if IS_ENABLED(CONFIG_SUNRPC_DEBUG) block.

static struct svc_xprt *svc_rdma_accept(struct svc_xprt *xprt)
{
....
RPC_IFDEBUG(struct sockaddr *sap);
...
#if IS_ENABLED(CONFIG_SUNRPC_DEBUG)
dprintk("svcrdma: new connection accepted on device %s:\n", dev->name);
sap = (struct sockaddr *)&newxprt->sc_cm_id->route.addr.src_addr;
dprintk(" local address : %pIS:%u\n", sap, rpc_get_port(sap));
sap = (struct sockaddr *)&newxprt->sc_cm_id->route.addr.dst_addr;
dprintk(" remote address : %pIS:%u\n", sap, rpc_get_port(sap));
dprintk(" max_sge : %d\n", newxprt->sc_max_send_sges);
dprintk(" sq_depth : %d\n", newxprt->sc_sq_depth);
dprintk(" rdma_rw_ctxs : %d\n", ctxts);
dprintk(" max_requests : %d\n", newxprt->sc_max_requests);
dprintk(" ord : %d\n", conn_param.initiator_depth);
#endif
...
}

The refactor in fs/nfsd/nfsfh.c will look like this:

static __be32 nfsd_setuser_and_check_port(struct svc_rqst *rqstp,
struct svc_cred *cred,
struct svc_export *exp)
{
    /* Check if the request originated from a secure port. */
    if (rqstp && !nfsd_originating_port_ok(rqstp, cred, exp)) {
       RPC_IFDEBUG(char buf[RPC_MAX_ADDRBUFLEN]);
+#if IS_ENABLED(CONFIG_SUNRPC_DEBUG)
       dprintk("nfsd: request from insecure port %s!\n",
       svc_print_addr(rqstp, buf, sizeof(buf)));
+#endif
       return nfserr_perm;
    }

    /* Set user creds for this exportpoint */
    return nfserrno(nfsd_setuser(cred, exp));
}

This fix, along with the macro simplification suggested by David, will
be included in v5.

[1] https://lore.kernel.org/all/69a2e269.050a0220.3a55be.003e.GAE@xxxxxxxxxx/

Best regards,
Sean