| Message ID | 20240618143640.169194-1-dmantipov@yandex.ru (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ceph: avoid call to strlen() in ceph_mds_auth_match() | expand |
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
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,
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,
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); } /*
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(-)