Re: [PATCH v2] nfsd: Avoid strlen conflict in nfsd4_encode_components_esc()

[Chuck Lever <chuck.lever@oracle.com>]

On 9/28/25 4:29 PM, NeilBrown wrote:
> On Mon, 29 Sep 2025, 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;
>>         |             ^~~~~~
>>
>> This dprintk() instance is not particularly useful, so just remove it
>> altogether.
>>
>> At the same time, rename the strlen local variable to avoid any
>> potential conflicts with strlen().
>>
>> Fixes: ec7d8e68ef0e ("sunrpc: add a Kconfig option to redirect dfprintk() output to trace buffer")
>> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
>> ---
>> Changes in v2:
>> - Remove dprintk() to remove usage of strlen()
>> - Rename local strlen variable to avoid potential conflict in the future
>> - Link to v1: https://patch.msgid.link/20250925-nfsd-fix-trace-printk-strlen-error-v1-1-1360530e4c6b@kernel.org
>> ---
>>  fs/nfsd/nfs4xdr.c | 10 ++++------
>>  1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
>> index ea91bad4eee2..9fe8a413f688 100644
>> --- a/fs/nfsd/nfs4xdr.c
>> +++ b/fs/nfsd/nfs4xdr.c
>> @@ -2640,11 +2640,9 @@ static __be32 nfsd4_encode_components_esc(struct xdr_stream *xdr, char sep,
>>  	__be32 *p;
>>  	__be32 pathlen;
>>  	int pathlen_offset;
>> -	int strlen, count=0;
>> +	int str_len, count=0;
>>  	char *str, *end, *next;
>>  
>> -	dprintk("nfsd4_encode_components(%s)\n", components);
>> -
>>  	pathlen_offset = xdr->buf->len;
>>  	p = xdr_reserve_space(xdr, 4);
>>  	if (!p)
>> @@ -2670,9 +2668,9 @@ static __be32 nfsd4_encode_components_esc(struct xdr_stream *xdr, char sep,
>>  			for (; *end && (*end != sep); end++)
>>  				/* find sep or end of string */;
>>  
>> -		strlen = end - str;
>> -		if (strlen) {
>> -			if (xdr_stream_encode_opaque(xdr, str, strlen) < 0)
>> +		str_len = end - str;
>> +		if (str_len) {
>> +			if (xdr_stream_encode_opaque(xdr, str, str_len) < 0)
>>  				return nfserr_resource;
> 
> I probably should have said something earlier, and this is definitely
> bike-shedding material, but .... "str_len" is not a whole lot nicer than
> "strlen" (or "i") ...
> 
> 
>    if (end > str) {
> 	if (xdr_stream_encode_opaque(xdr, str, end - str) < 0)
> 
> ??

"len" is actually the typical variable name used in such cases. But I
didn't want to bug Nathan for a resend.


-- 
Chuck Lever