| Message ID | 20241210163723.20137-1-dmantipov@yandex.ru (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | ocfs2: avoid extra calls to strlen() | expand |
On 2024/12/11 00:37, Dmitry Antipov wrote: > Since 'ocfs2_sprintf_system_inode_name()' returns the > number of characters emitted, avoid extra calls to > 'strlen()' where appropriate. Compile tested only. > Ummm... It seems this is not an equivalent replace. snprintf() returns the number of characters which would be generated, while strlen() returns the actually written characters. Thanks, Joseph > Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru> > --- > fs/ocfs2/ioctl.c | 21 +++++++++------------ > fs/ocfs2/move_extents.c | 9 +++++---- > fs/ocfs2/sysfile.c | 12 ++++++------ > 3 files changed, 20 insertions(+), 22 deletions(-) > > diff --git a/fs/ocfs2/ioctl.c b/fs/ocfs2/ioctl.c > index 71beef7f8a60..cec70bf4e478 100644 > --- a/fs/ocfs2/ioctl.c > +++ b/fs/ocfs2/ioctl.c > @@ -329,7 +329,7 @@ static int ocfs2_info_handle_freeinode(struct inode *inode, > u32 i; > u64 blkno = -1; > char namebuf[40]; > - int status, type = INODE_ALLOC_SYSTEM_INODE; > + int len, status, type = INODE_ALLOC_SYSTEM_INODE; > struct ocfs2_info_freeinode *oifi = NULL; > struct ocfs2_super *osb = OCFS2_SB(inode->i_sb); > struct inode *inode_alloc = NULL; > @@ -358,12 +358,11 @@ static int ocfs2_info_handle_freeinode(struct inode *inode, > goto bail; > } > } else { > - ocfs2_sprintf_system_inode_name(namebuf, > - sizeof(namebuf), > - type, i); > + len = ocfs2_sprintf_system_inode_name(namebuf, > + sizeof(namebuf), > + type, i); > status = ocfs2_lookup_ino_from_name(osb->sys_root_inode, > - namebuf, > - strlen(namebuf), > + namebuf, len, > &blkno); > if (status < 0) { > status = -ENOENT; > @@ -616,7 +615,7 @@ static int ocfs2_info_handle_freefrag(struct inode *inode, > { > u64 blkno = -1; > char namebuf[40]; > - int status, type = GLOBAL_BITMAP_SYSTEM_INODE; > + int len, status, type = GLOBAL_BITMAP_SYSTEM_INODE; > > struct ocfs2_info_freefrag *oiff; > struct ocfs2_super *osb = OCFS2_SB(inode->i_sb); > @@ -651,12 +650,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); > + 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 f9d6a4f9ca92..c1b7c6cee95d 100644 > --- a/fs/ocfs2/move_extents.c > +++ b/fs/ocfs2/move_extents.c > @@ -364,7 +364,7 @@ static int ocfs2_find_victim_alloc_group(struct inode *inode, > int *vict_bit, > struct buffer_head **ret_bh) > { > - int ret, i, bits_per_unit = 0; > + int len, ret, i, bits_per_unit = 0; > u64 blkno; > char namebuf[40]; > > @@ -375,9 +375,10 @@ 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); > + len = ocfs2_sprintf_system_inode_name(namebuf, sizeof(namebuf), > + type, slot); > + ret = ocfs2_lookup_ino_from_name(osb->sys_root_inode, > + namebuf, len, &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; > }
On 12/13/24 1:09 PM, Joseph Qi wrote: > snprintf() returns the number of characters which would be generated, > while strlen() returns the actually written characters. Well, if these two mismatch, this means that the result was (most likely) truncated because an output buffer was too small. IMHO it may be helpful to have an extra debugging check to catch these suspicious cases. Dmitry
diff --git a/fs/ocfs2/ioctl.c b/fs/ocfs2/ioctl.c index 71beef7f8a60..cec70bf4e478 100644 --- a/fs/ocfs2/ioctl.c +++ b/fs/ocfs2/ioctl.c @@ -329,7 +329,7 @@ static int ocfs2_info_handle_freeinode(struct inode *inode, u32 i; u64 blkno = -1; char namebuf[40]; - int status, type = INODE_ALLOC_SYSTEM_INODE; + int len, status, type = INODE_ALLOC_SYSTEM_INODE; struct ocfs2_info_freeinode *oifi = NULL; struct ocfs2_super *osb = OCFS2_SB(inode->i_sb); struct inode *inode_alloc = NULL; @@ -358,12 +358,11 @@ static int ocfs2_info_handle_freeinode(struct inode *inode, goto bail; } } else { - ocfs2_sprintf_system_inode_name(namebuf, - sizeof(namebuf), - type, i); + len = ocfs2_sprintf_system_inode_name(namebuf, + sizeof(namebuf), + type, i); status = ocfs2_lookup_ino_from_name(osb->sys_root_inode, - namebuf, - strlen(namebuf), + namebuf, len, &blkno); if (status < 0) { status = -ENOENT; @@ -616,7 +615,7 @@ static int ocfs2_info_handle_freefrag(struct inode *inode, { u64 blkno = -1; char namebuf[40]; - int status, type = GLOBAL_BITMAP_SYSTEM_INODE; + int len, status, type = GLOBAL_BITMAP_SYSTEM_INODE; struct ocfs2_info_freefrag *oiff; struct ocfs2_super *osb = OCFS2_SB(inode->i_sb); @@ -651,12 +650,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); + 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 f9d6a4f9ca92..c1b7c6cee95d 100644 --- a/fs/ocfs2/move_extents.c +++ b/fs/ocfs2/move_extents.c @@ -364,7 +364,7 @@ static int ocfs2_find_victim_alloc_group(struct inode *inode, int *vict_bit, struct buffer_head **ret_bh) { - int ret, i, bits_per_unit = 0; + int len, ret, i, bits_per_unit = 0; u64 blkno; char namebuf[40]; @@ -375,9 +375,10 @@ 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); + len = ocfs2_sprintf_system_inode_name(namebuf, sizeof(namebuf), + type, slot); + ret = ocfs2_lookup_ino_from_name(osb->sys_root_inode, + namebuf, len, &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; }
Since 'ocfs2_sprintf_system_inode_name()' returns the number of characters emitted, avoid extra calls to 'strlen()' where appropriate. Compile tested only. Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru> --- fs/ocfs2/ioctl.c | 21 +++++++++------------ fs/ocfs2/move_extents.c | 9 +++++---- fs/ocfs2/sysfile.c | 12 ++++++------ 3 files changed, 20 insertions(+), 22 deletions(-)