<div dir="auto">Hi Gao,<div dir="auto"><br></div><div dir="auto">Sorry I didn't pull the latest tree. I will do the necessary. </div><div dir="auto">Anyways, don't you think it will be cleaner to have a switch case statement rather than if-else statement.</div><div dir="auto"><br></div><div dir="auto">--Pratik</div><div dir="auto"><div dir="auto"><div dir="auto"><br></div><div dir="auto"><br></div></div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, 29 Aug, 2019, 7:27 PM Gao Xiang, <<a href="mailto:gaoxiang25@huawei.com">gaoxiang25@huawei.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Pratik,<br>
<br>
On Thu, Aug 29, 2019 at 06:38:13PM +0530, Pratik Shinde wrote:<br>
> while filling the linux inode, using switch-case statement to check<br>
> the type of inode.<br>
> switch-case statement looks more clean.<br>
> <br>
> Signed-off-by: Pratik Shinde <<a href="mailto:pratikshinde320@gmail.com" target="_blank" rel="noreferrer">pratikshinde320@gmail.com</a>><br>
<br>
No, that is not the case, see __ext4_iget() in fs/ext4/inode.c.<br>
and could you write patches based on latest staging tree?<br>
erofs is now in "fs/" rather than "drivers/staging".<br>
and I will review it then.<br>
<br>
p.s. if someone argues here or there, there will be endless<br>
issues since there is no standard at all.<br>
<br>
Thanks,<br>
Gao Xiang<br>
<br>
> ---<br>
>  drivers/staging/erofs/inode.c | 18 ++++++++++++------<br>
>  1 file changed, 12 insertions(+), 6 deletions(-)<br>
> <br>
> diff --git a/drivers/staging/erofs/inode.c b/drivers/staging/erofs/inode.c<br>
> index 4c3d8bf..2d2d545 100644<br>
> --- a/drivers/staging/erofs/inode.c<br>
> +++ b/drivers/staging/erofs/inode.c<br>
> @@ -190,22 +190,28 @@ static int fill_inode(struct inode *inode, int isdir)<br>
>       err = read_inode(inode, data + ofs);<br>
>       if (!err) {<br>
>               /* setup the new inode */<br>
> -             if (S_ISREG(inode->i_mode)) {<br>
> +             switch (inode->i_mode & S_IFMT) {<br>
> +             case S_IFREG:<br>
>                       inode->i_op = &erofs_generic_iops;<br>
>                       inode->i_fop = &generic_ro_fops;<br>
> -             } else if (S_ISDIR(inode->i_mode)) {<br>
> +                     break;<br>
> +             case S_IFDIR:<br>
>                       inode->i_op = &erofs_dir_iops;<br>
>                       inode->i_fop = &erofs_dir_fops;<br>
> -             } else if (S_ISLNK(inode->i_mode)) {<br>
> +                     break;<br>
> +             case S_IFLNK:<br>
>                       /* by default, page_get_link is used for symlink */<br>
>                       inode->i_op = &erofs_symlink_iops;<br>
>                       inode_nohighmem(inode);<br>
> -             } else if (S_ISCHR(inode->i_mode) || S_ISBLK(inode->i_mode) ||<br>
> -                     S_ISFIFO(inode->i_mode) || S_ISSOCK(inode->i_mode)) {<br>
> +                     break;<br>
> +             case S_IFCHR:<br>
> +             case S_IFBLK:<br>
> +             case S_IFIFO:<br>
> +             case S_IFSOCK:<br>
>                       inode->i_op = &erofs_generic_iops;<br>
>                       init_special_inode(inode, inode->i_mode, inode->i_rdev);<br>
>                       goto out_unlock;<br>
> -             } else {<br>
> +             default:<br>
>                       err = -EIO;<br>
>                       goto out_unlock;<br>
>               }<br>
> -- <br>
> 2.9.3<br>
> <br>
</blockquote></div>