linux-kernel - Re: [PATCH] fs: ufs: Remove switch statement from ufs_set_de

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20181020222637.GA3477@bombadil.infradead.org>
Date:   Sat, 20 Oct 2018 15:26:37 -0700
From:   Matthew Wilcox <willy@...radead.org>
To:     Phillip Potter <phil@...lpotter.co.uk>
Cc:     dushistov@...l.ru, viro@...iv.linux.org.uk,
        David.Laight@...lab.com, linux-kernel@...r.kernel.org,
        linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH] fs: ufs: Remove switch statement from ufs_set_de_type
 function

On Sat, Oct 20, 2018 at 11:09:57PM +0100, Phillip Potter wrote:
> Remove switch statement from ufs_set_de_type function in fs/ufs/util.h
> header and replace with simple assignment. For each case, S_IFx >> 12
> is equal to DT_x, so in valid cases (mode & S_IFMT) >> 12 should give
> us the correct file type. It is expected that for *nix compatibility
> reasons, the relation between S_IFx and DT_x will not change. For
> cases where the mode is invalid, upper layer validation catches this
> anyway, so this improves readability and arguably performance by
> assigning (mode & S_IFMT) >> 12 directly without any jump table
> or conditional logic.

Shouldn't we also do this for other filesystems?  A quick scan suggests
someone should at least look at xfs, ubifs, squashfs, romfs, ocfs2,
nilfs2, hfsplus, f2fs, ext4, ext2, exofs, coda, cifs & btrfs.

> Signed-off-by: Phillip Potter <phil@...lpotter.co.uk>
> ---
>  fs/ufs/util.h | 30 ++----------------------------
>  1 file changed, 2 insertions(+), 28 deletions(-)
> 
> diff --git a/fs/ufs/util.h b/fs/ufs/util.h
> index 1fd3011ea623..7e0c0878b9f9 100644
> --- a/fs/ufs/util.h
> +++ b/fs/ufs/util.h
> @@ -16,6 +16,7 @@
>   * some useful macros
>   */
>  #define in_range(b,first,len)	((b)>=(first)&&(b)<(first)+(len))
> +#define S_SHIFT			12
>  
>  /*
>   * functions used for retyping
> @@ -158,34 +159,7 @@ ufs_set_de_type(struct super_block *sb, struct ufs_dir_entry *de, int mode)
>  	if ((UFS_SB(sb)->s_flags & UFS_DE_MASK) != UFS_DE_44BSD)
>  		return;
>  
> -	/*
> -	 * TODO turn this into a table lookup
> -	 */
> -	switch (mode & S_IFMT) {
> -	case S_IFSOCK:
> -		de->d_u.d_44.d_type = DT_SOCK;
> -		break;
> -	case S_IFLNK:
> -		de->d_u.d_44.d_type = DT_LNK;
> -		break;
> -	case S_IFREG:
> -		de->d_u.d_44.d_type = DT_REG;
> -		break;
> -	case S_IFBLK:
> -		de->d_u.d_44.d_type = DT_BLK;
> -		break;
> -	case S_IFDIR:
> -		de->d_u.d_44.d_type = DT_DIR;
> -		break;
> -	case S_IFCHR:
> -		de->d_u.d_44.d_type = DT_CHR;
> -		break;
> -	case S_IFIFO:
> -		de->d_u.d_44.d_type = DT_FIFO;
> -		break;
> -	default:
> -		de->d_u.d_44.d_type = DT_UNKNOWN;
> -	}
> +	de->d_u.d_44.d_type = (mode & S_IFMT) >> S_SHIFT;
>  }
>  
>  static inline u32
> -- 
> 2.17.2
>