[PATCH] ceph: avoid call to strlen() in ceph_mds_auth_match()

[PATCH] ceph: avoid call to strlen() in ceph_mds_auth_match()

[Dmitry Antipov]

Since 'snprintf()' returns the number of characters emitted,
an extra call to 'strlen()' in 'ceph_mds_auth_match()' may
be dropped. Compile tested only.

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
 fs/ceph/mds_client.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index c2157f6e0c69..7224283046a7 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -5665,9 +5665,9 @@ static int ceph_mds_auth_match(struct ceph_mds_client *mdsc,
 				if (!_tpath)
 					return -ENOMEM;
 				/* remove the leading '/' */
-				snprintf(_tpath, n, "%s/%s", spath + 1, tpath);
+				tlen = snprintf(_tpath, n, "%s/%s",
+						spath + 1, tpath);
 				free_tpath = true;
-				tlen = strlen(_tpath);
 			}
 
 			/*
-- 
2.45.2

Re: [PATCH] ceph: avoid call to strlen() in ceph_mds_auth_match()

[Xiubo Li]

On 6/18/24 22:36, Dmitry Antipov wrote:
> Since 'snprintf()' returns the number of characters emitted,
> an extra call to 'strlen()' in 'ceph_mds_auth_match()' may
> be dropped. Compile tested only.
>
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
> ---
>   fs/ceph/mds_client.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> index c2157f6e0c69..7224283046a7 100644
> --- a/fs/ceph/mds_client.c
> +++ b/fs/ceph/mds_client.c
> @@ -5665,9 +5665,9 @@ static int ceph_mds_auth_match(struct ceph_mds_client *mdsc,
>   				if (!_tpath)
>   					return -ENOMEM;
>   				/* remove the leading '/' */
> -				snprintf(_tpath, n, "%s/%s", spath + 1, tpath);
> +				tlen = snprintf(_tpath, n, "%s/%s",
> +						spath + 1, tpath);
>   				free_tpath = true;
> -				tlen = strlen(_tpath);
>   			}
>   
>   			/*

Both snprintf and strlen will return the string length without the 
trailing null. So this change LGTM.

Applied to the 'testing' branch and will run the tests.

Reviewed-by: Xiubo Li <xiubli@redhat.com>

Thanks

Re: [PATCH] ceph: avoid call to strlen() in ceph_mds_auth_match()

[Luis Henriques]

On Tue 18 Jun 2024 05:36:40 PM +03, Dmitry Antipov wrote;

> Since 'snprintf()' returns the number of characters emitted,
> an extra call to 'strlen()' in 'ceph_mds_auth_match()' may
> be dropped. Compile tested only.
>
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
> ---
>  fs/ceph/mds_client.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> index c2157f6e0c69..7224283046a7 100644
> --- a/fs/ceph/mds_client.c
> +++ b/fs/ceph/mds_client.c
> @@ -5665,9 +5665,9 @@ static int ceph_mds_auth_match(struct ceph_mds_client *mdsc,
>  				if (!_tpath)
>  					return -ENOMEM;
>  				/* remove the leading '/' */
> -				snprintf(_tpath, n, "%s/%s", spath + 1, tpath);
> +				tlen = snprintf(_tpath, n, "%s/%s",
> +						spath + 1, tpath);
>  				free_tpath = true;
> -				tlen = strlen(_tpath);
>  			}

Unless I'm missing something, this patch is incorrect.  snprintf() may not
return the actual string length *if* the output is truncated.  For
example:

	snprintf(str, 5, "%s", "0123456789");

snprintf() will return 10, while strlen(str) will return 4.

Cheers,
-- 
Luís

Re: [PATCH] ceph: avoid call to strlen() in ceph_mds_auth_match()

[Xiubo Li]

On 6/19/24 20:37, Luis Henriques wrote:
> On Tue 18 Jun 2024 05:36:40 PM +03, Dmitry Antipov wrote;
>
>> Since 'snprintf()' returns the number of characters emitted,
>> an extra call to 'strlen()' in 'ceph_mds_auth_match()' may
>> be dropped. Compile tested only.
>>
>> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
>> ---
>>   fs/ceph/mds_client.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
>> index c2157f6e0c69..7224283046a7 100644
>> --- a/fs/ceph/mds_client.c
>> +++ b/fs/ceph/mds_client.c
>> @@ -5665,9 +5665,9 @@ static int ceph_mds_auth_match(struct ceph_mds_client *mdsc,
>>   				if (!_tpath)
>>   					return -ENOMEM;
>>   				/* remove the leading '/' */
>> -				snprintf(_tpath, n, "%s/%s", spath + 1, tpath);
>> +				tlen = snprintf(_tpath, n, "%s/%s",
>> +						spath + 1, tpath);
>>   				free_tpath = true;
>> -				tlen = strlen(_tpath);
>>   			}
> Unless I'm missing something, this patch is incorrect.  snprintf() may not
> return the actual string length *if* the output is truncated.  For
> example:
>
> 	snprintf(str, 5, "%s", "0123456789");
>
> snprintf() will return 10, while strlen(str) will return 4.

Yeah, you are correct.

Thanks Luis.


> Cheers,