lkml.org 
[lkml]   [2023]   [Nov]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH -next V2] erofs: code clean up for function erofs_read_inode()
From
Hi,

On 2023/11/10 03:48, WoZ1zh1 wrote:
> Because variables "die" and "copied" only appear in case
> EROFS_INODE_LAYOUT_EXTENDED, move them from the outer space into this
> case. Also, call "kfree(copied)" earlier to avoid double free in the
> "error_out" branch. Some cleanups, no logic changes.
>
> Signed-off-by: WoZ1zh1 <wozizhi@huawei.com>

Please help use your real name here...

> ---
> fs/erofs/inode.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/erofs/inode.c b/fs/erofs/inode.c
> index b8ad05b4509d..a388c93eec34 100644
> --- a/fs/erofs/inode.c
> +++ b/fs/erofs/inode.c
> @@ -19,7 +19,6 @@ static void *erofs_read_inode(struct erofs_buf *buf,
> erofs_blk_t blkaddr, nblks = 0;
> void *kaddr;
> struct erofs_inode_compact *dic;
> - struct erofs_inode_extended *die, *copied = NULL;
> unsigned int ifmt;
> int err;
>
> @@ -53,6 +52,8 @@ static void *erofs_read_inode(struct erofs_buf *buf,
>
> switch (erofs_inode_version(ifmt)) {
> case EROFS_INODE_LAYOUT_EXTENDED:
> + struct erofs_inode_extended *die, *copied = NULL;

Thanks for the patch, but in my own opinion:

1) It doesn't simplify the code

2) We'd like to avoid defining variables like this (in the
switch block), and I even don't think this patch can compile.

3) The logic itself is also broken...

Thanks,
Gao Xiang

> +
> vi->inode_isize = sizeof(struct erofs_inode_extended);
> /* check if the extended inode acrosses block boundary */
> if (*ofs + vi->inode_isize <= sb->s_blocksize) {
> @@ -98,6 +99,7 @@ static void *erofs_read_inode(struct erofs_buf *buf,
> inode->i_rdev = 0;
> break;
> default:
> + kfree(copied);
> goto bogusimode;
> }
> i_uid_write(inode, le32_to_cpu(die->i_uid));
> @@ -117,7 +119,6 @@ static void *erofs_read_inode(struct erofs_buf *buf,
> /* fill chunked inode summary info */
> vi->chunkformat = le16_to_cpu(die->i_u.c.format);
> kfree(copied);
> - copied = NULL;
> break;
> case EROFS_INODE_LAYOUT_COMPACT:
> vi->inode_isize = sizeof(struct erofs_inode_compact);
> @@ -197,7 +198,6 @@ static void *erofs_read_inode(struct erofs_buf *buf,
> err = -EFSCORRUPTED;
> err_out:
> DBG_BUGON(1);
> - kfree(copied);
> erofs_put_metabuf(buf);
> return ERR_PTR(err);
> }

\
 
 \ /
  Last update: 2023-11-20 13:54    [W:1.403 / U:0.072 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site