* Re: [PATCH v2] nfsd: Avoid strlen conflict in nfsd4_encode_components_esc()
2025-09-28 19:14 [PATCH v2] nfsd: Avoid strlen conflict in nfsd4_encode_components_esc() Nathan Chancellor
@ 2025-09-28 19:56 ` Jeff Layton
2025-09-28 20:13 ` Chuck Lever
2025-09-28 23:29 ` NeilBrown
2 siblings, 0 replies; 7+ messages in thread
From: Jeff Layton @ 2025-09-28 19:56 UTC (permalink / raw)
To: Nathan Chancellor, Chuck Lever, Anna Schumaker
Cc: NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey, Simon Horman,
linux-nfs, patches
On Sun, 2025-09-28 at 15:14 -0400, 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;
> count++;
> } else
>
> ---
> base-commit: 3fadfaec904dffab02ebf63dd9c2ae8fa15c6d32
> change-id: 20250925-nfsd-fix-trace-printk-strlen-error-2a24413eb186
>
> Best regards,
> --
> Nathan Chancellor <nathan@kernel.org>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2] nfsd: Avoid strlen conflict in nfsd4_encode_components_esc()
2025-09-28 19:14 [PATCH v2] nfsd: Avoid strlen conflict in nfsd4_encode_components_esc() Nathan Chancellor
2025-09-28 19:56 ` Jeff Layton
@ 2025-09-28 20:13 ` Chuck Lever
2025-09-28 23:29 ` NeilBrown
2 siblings, 0 replies; 7+ messages in thread
From: Chuck Lever @ 2025-09-28 20:13 UTC (permalink / raw)
To: Nathan Chancellor, Jeff Layton, Anna Schumaker
Cc: NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey, Simon Horman,
linux-nfs, patches
On 9/28/25 12:14 PM, 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;
> count++;
> } else
>
> ---
> base-commit: 3fadfaec904dffab02ebf63dd9c2ae8fa15c6d32
> change-id: 20250925-nfsd-fix-trace-printk-strlen-error-2a24413eb186
>
> Best regards,
> --
> Nathan Chancellor <nathan@kernel.org>
>
Since "sunrpc: add a Kconfig option to redirect dfprintk() output to
trace buffer" is going through the NFS client tree, Anna will have to
take this one.
Reviewed-by: Chuck Lever <chuck.lever@oracle.com>
--
Chuck Lever
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2] nfsd: Avoid strlen conflict in nfsd4_encode_components_esc()
2025-09-28 19:14 [PATCH v2] nfsd: Avoid strlen conflict in nfsd4_encode_components_esc() Nathan Chancellor
2025-09-28 19:56 ` Jeff Layton
2025-09-28 20:13 ` Chuck Lever
@ 2025-09-28 23:29 ` NeilBrown
2025-09-29 13:05 ` Chuck Lever
2 siblings, 1 reply; 7+ messages in thread
From: NeilBrown @ 2025-09-28 23:29 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Chuck Lever, Jeff Layton, Anna Schumaker, Olga Kornievskaia,
Dai Ngo, Tom Talpey, Simon Horman, linux-nfs, patches,
Nathan Chancellor
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)
??
NeilBrown
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2] nfsd: Avoid strlen conflict in nfsd4_encode_components_esc()
2025-09-28 23:29 ` NeilBrown
@ 2025-09-29 13:05 ` Chuck Lever
2025-09-29 18:11 ` Nathan Chancellor
0 siblings, 1 reply; 7+ messages in thread
From: Chuck Lever @ 2025-09-29 13:05 UTC (permalink / raw)
To: NeilBrown, Nathan Chancellor
Cc: Jeff Layton, Anna Schumaker, Olga Kornievskaia, Dai Ngo,
Tom Talpey, Simon Horman, linux-nfs, patches
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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2] nfsd: Avoid strlen conflict in nfsd4_encode_components_esc()
2025-09-29 13:05 ` Chuck Lever
@ 2025-09-29 18:11 ` Nathan Chancellor
2025-09-30 5:32 ` NeilBrown
0 siblings, 1 reply; 7+ messages in thread
From: Nathan Chancellor @ 2025-09-29 18:11 UTC (permalink / raw)
To: Chuck Lever
Cc: NeilBrown, Jeff Layton, Anna Schumaker, Olga Kornievskaia,
Dai Ngo, Tom Talpey, Simon Horman, linux-nfs, patches
On Mon, Sep 29, 2025 at 09:05:15AM -0400, Chuck Lever wrote:
> On 9/28/25 4:29 PM, NeilBrown wrote:
> > On Mon, 29 Sep 2025, Nathan Chancellor wrote:
...
> >> 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.
If "len" is truly preferred, I do not mind sending a v3 with such a
change. I had only kept "str" in the name because there is also
"pathlen" in this function.
Cheers,
Nathan
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2] nfsd: Avoid strlen conflict in nfsd4_encode_components_esc()
2025-09-29 18:11 ` Nathan Chancellor
@ 2025-09-30 5:32 ` NeilBrown
0 siblings, 0 replies; 7+ messages in thread
From: NeilBrown @ 2025-09-30 5:32 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Chuck Lever, Jeff Layton, Anna Schumaker, Olga Kornievskaia,
Dai Ngo, Tom Talpey, Simon Horman, linux-nfs, patches
On Tue, 30 Sep 2025, Nathan Chancellor wrote:
> On Mon, Sep 29, 2025 at 09:05:15AM -0400, Chuck Lever wrote:
> > On 9/28/25 4:29 PM, NeilBrown wrote:
> > > On Mon, 29 Sep 2025, Nathan Chancellor wrote:
> ...
> > >> 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.
>
> If "len" is truly preferred, I do not mind sending a v3 with such a
> change. I had only kept "str" in the name because there is also
> "pathlen" in this function.
If you are going to send a v3 I would much prefer there were no variable
at all. Just use
if (end > str) {
if (xdr_stream_encode_opaque(xdr, str, end - str) < 0)
...
If we wanted a 'len' variable then I would change "str" to "component"
or some abbreviation there-of and use "componentlen" (or e.g. cmpnt,
cmpntlen). This function is encoding "components" so using suitably
named variables to focus the reader on which bits are the "components"
seems wise.
But as I say, we don't need a "len" variable, so I wouldn't bother
renaming "str" to "cmpnt" at this stage.
Thanks,
NeilBrown
^ permalink raw reply [flat|nested] 7+ messages in thread