Re: [PATCH] nfsd: Move strlen declaration in nfsd4_encode_components_esc()

[Jeff Layton <jlayton@kernel.org>]

On Thu, 2025-09-25 at 09:43 -0400, Chuck Lever wrote:
> On 9/25/25 5:14 AM, Nathan Chancellor wrote:
> > There is an error building nfs4xdr.c with CONFIG_SUNRPC_DEBUG_TRACE=y
> > and CONFIG_FORTIFY_SOURCE=n due to the local variable strlen conflicting
> > with the function strlen():
> > 
> >   In file included from include/linux/cpumask.h:11,
> >                    from arch/x86/include/asm/paravirt.h:21,
> >                    from arch/x86/include/asm/irqflags.h:102,
> >                    from include/linux/irqflags.h:18,
> >                    from include/linux/spinlock.h:59,
> >                    from include/linux/mmzone.h:8,
> >                    from include/linux/gfp.h:7,
> >                    from include/linux/slab.h:16,
> >                    from fs/nfsd/nfs4xdr.c:37:
> >   fs/nfsd/nfs4xdr.c: In function 'nfsd4_encode_components_esc':
> >   include/linux/kernel.h:321:46: error: called object 'strlen' is not a function or function pointer
> >     321 |                 __trace_puts(_THIS_IP_, str, strlen(str));              \
> >         |                                              ^~~~~~
> >   include/linux/kernel.h:265:17: note: in expansion of macro 'trace_puts'
> >     265 |                 trace_puts(fmt);                        \
> >         |                 ^~~~~~~~~~
> >   include/linux/sunrpc/debug.h:34:41: note: in expansion of macro 'trace_printk'
> >      34 | #  define __sunrpc_printk(fmt, ...)     trace_printk(fmt, ##__VA_ARGS__)
> >         |                                         ^~~~~~~~~~~~
> >   include/linux/sunrpc/debug.h:42:17: note: in expansion of macro '__sunrpc_printk'
> >      42 |                 __sunrpc_printk(fmt, ##__VA_ARGS__);                    \
> >         |                 ^~~~~~~~~~~~~~~
> >   include/linux/sunrpc/debug.h:25:9: note: in expansion of macro 'dfprintk'
> >      25 |         dfprintk(FACILITY, fmt, ##__VA_ARGS__)
> >         |         ^~~~~~~~
> >   fs/nfsd/nfs4xdr.c:2646:9: note: in expansion of macro 'dprintk'
> >    2646 |         dprintk("nfsd4_encode_components(%s)\n", components);
> >         |         ^~~~~~~
> >   fs/nfsd/nfs4xdr.c:2643:13: note: declared here
> >    2643 |         int strlen, count=0;
> >         |             ^~~~~~
> > 
> > Move the declaration of strlen into the while loop (as that is the only
> > place where it is used), which is after the call to dprintk, to clear up
> > the error.
> > 
> > Fixes: ec7d8e68ef0e ("sunrpc: add a Kconfig option to redirect dfprintk() output to trace buffer")
> > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> > ---
> > This solution may be too subtle but given that dprintk() seems to be on
> > its way out, maybe it is fine. An alternative would be a rename such as
> > str_len but there is some symmetry with pathlen so I opted for this one
> > up front.
> > ---
> >  fs/nfsd/nfs4xdr.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> > index ea91bad4eee2..580bfa8011c7 100644
> > --- a/fs/nfsd/nfs4xdr.c
> > +++ b/fs/nfsd/nfs4xdr.c
> > @@ -2640,7 +2640,7 @@ static __be32 nfsd4_encode_components_esc(struct xdr_stream *xdr, char sep,
> >  	__be32 *p;
> >  	__be32 pathlen;
> >  	int pathlen_offset;
> > -	int strlen, count=0;
> > +	int count=0;
> >  	char *str, *end, *next;
> >  
> >  	dprintk("nfsd4_encode_components(%s)\n", components);
> > @@ -2654,6 +2654,7 @@ static __be32 nfsd4_encode_components_esc(struct xdr_stream *xdr, char sep,
> >  	end = str = components;
> >  	while (*end) {
> >  		bool found_esc = false;
> > +		int strlen;
> >  
> >  		/* try to parse as esc_start, ..., esc_end, sep */
> >  		if (*str == esc_enter) {
> > 
> > ---
> > base-commit: 3fadfaec904dffab02ebf63dd9c2ae8fa15c6d32
> > change-id: 20250925-nfsd-fix-trace-printk-strlen-error-2a24413eb186
> > 
> > Best regards,
> > --  
> > Nathan Chancellor <nathan@kernel.org>
> > 
> 
> Would anyone be heartbroken if this patch removed the dprintk call site?
> 
> I think renaming the strlen variable to a name with a lower collision
> risk would be sensible as well.
> 

Fine with me on both counts. No point in overloading a well known
function name here.

-- 
Jeff Layton <jlayton@kernel.org>