Re: [PATCH] ocfs2: avoid extra calls to strlen() after ocfs2_sprintf_system_inode_name()

[Joel Becker <jlbec@evilplan.org>]

Reviewed-by: Joel Becker <jlbec@evilplan.org>

On Mon, Sep 15, 2025 at 03:19:06PM +0300, Dmitry Antipov wrote:
> Since 'ocfs2_sprintf_system_inode_name()' uses 'snprintf()' and returns
> the number of characters emitted, callers of the former are better to
> use that return value instead of an explicit calls to 'strlen()'.
> 
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
> ---
>  fs/ocfs2/ioctl.c        | 18 +++++++-----------
>  fs/ocfs2/move_extents.c |  6 +++---
>  fs/ocfs2/sysfile.c      | 12 ++++++------
>  3 files changed, 16 insertions(+), 20 deletions(-)
> 
> diff --git a/fs/ocfs2/ioctl.c b/fs/ocfs2/ioctl.c
> index db14c92302a1..b6864602814c 100644
> --- a/fs/ocfs2/ioctl.c
> +++ b/fs/ocfs2/ioctl.c
> @@ -358,13 +358,11 @@ static int ocfs2_info_handle_freeinode(struct inode *inode,
>  				goto bail;
>  			}
>  		} else {
> -			ocfs2_sprintf_system_inode_name(namebuf,
> -							sizeof(namebuf),
> -							type, i);
> +			int len = ocfs2_sprintf_system_inode_name(namebuf,
> +								  sizeof(namebuf),
> +								  type, i);
>  			status = ocfs2_lookup_ino_from_name(osb->sys_root_inode,
> -							    namebuf,
> -							    strlen(namebuf),
> -							    &blkno);
> +							    namebuf, len, &blkno);
>  			if (status < 0) {
>  				status = -ENOENT;
>  				goto bail;
> @@ -651,12 +649,10 @@ static int ocfs2_info_handle_freefrag(struct inode *inode,
>  			goto bail;
>  		}
>  	} else {
> -		ocfs2_sprintf_system_inode_name(namebuf, sizeof(namebuf), type,
> -						OCFS2_INVALID_SLOT);
> +		int len = ocfs2_sprintf_system_inode_name(namebuf, sizeof(namebuf),
> +							  type, OCFS2_INVALID_SLOT);
>  		status = ocfs2_lookup_ino_from_name(osb->sys_root_inode,
> -						    namebuf,
> -						    strlen(namebuf),
> -						    &blkno);
> +						    namebuf, len, &blkno);
>  		if (status < 0) {
>  			status = -ENOENT;
>  			goto bail;
> diff --git a/fs/ocfs2/move_extents.c b/fs/ocfs2/move_extents.c
> index cbe2f8ed8897..a2b4c3f2a71c 100644
> --- a/fs/ocfs2/move_extents.c
> +++ b/fs/ocfs2/move_extents.c
> @@ -375,9 +375,9 @@ static int ocfs2_find_victim_alloc_group(struct inode *inode,
>  	struct ocfs2_dinode *ac_dinode;
>  	struct ocfs2_group_desc *bg;
>  
> -	ocfs2_sprintf_system_inode_name(namebuf, sizeof(namebuf), type, slot);
> -	ret = ocfs2_lookup_ino_from_name(osb->sys_root_inode, namebuf,
> -					 strlen(namebuf), &blkno);
> +	i = ocfs2_sprintf_system_inode_name(namebuf, sizeof(namebuf), type, slot);
> +	ret = ocfs2_lookup_ino_from_name(osb->sys_root_inode, namebuf, i, &blkno);
> +
>  	if (ret) {
>  		ret = -ENOENT;
>  		goto out;
> diff --git a/fs/ocfs2/sysfile.c b/fs/ocfs2/sysfile.c
> index 53a945da873b..d53a6cc866be 100644
> --- a/fs/ocfs2/sysfile.c
> +++ b/fs/ocfs2/sysfile.c
> @@ -127,14 +127,14 @@ static struct inode * _ocfs2_get_system_file_inode(struct ocfs2_super *osb,
>  	char namebuf[40];
>  	struct inode *inode = NULL;
>  	u64 blkno;
> -	int status = 0;
> +	int len, status = 0;
>  
> -	ocfs2_sprintf_system_inode_name(namebuf,
> -					sizeof(namebuf),
> -					type, slot);
> +	len = ocfs2_sprintf_system_inode_name(namebuf,
> +					      sizeof(namebuf),
> +					      type, slot);
>  
> -	status = ocfs2_lookup_ino_from_name(osb->sys_root_inode, namebuf,
> -					    strlen(namebuf), &blkno);
> +	status = ocfs2_lookup_ino_from_name(osb->sys_root_inode,
> +					    namebuf, len, &blkno);
>  	if (status < 0) {
>  		goto bail;
>  	}
> -- 
> 2.51.0
> 

-- 

"Anything that is too stupid to be spoken is sung."  
        - Voltaire

			http://www.jlbec.org/
			jlbec@evilplan.org