From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-f65.google.com ([209.85.128.65]:36617 "EHLO mail-wm1-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727523AbeJVQhh (ORCPT ); Mon, 22 Oct 2018 12:37:37 -0400 Received: by mail-wm1-f65.google.com with SMTP id a8-v6so9313529wmf.1 for ; Mon, 22 Oct 2018 01:20:07 -0700 (PDT) Date: Mon, 22 Oct 2018 09:20:03 +0100 From: Phillip Potter To: Amir Goldstein Cc: willy@infradead.org, dushistov@mail.ru, viro@zeniv.linux.org.uk, David.Laight@aculab.com, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org Subject: Re: [PATCH] fs: ufs: Remove switch statement from ufs_set_de_type function Message-ID: <20181022082003.GA3318@pathfinder> References: <20181020220957.GA7267@pathfinder> <20181020222637.GA3477@bombadil.infradead.org> <20181021095710.GA3482@pathfinder> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Sun, Oct 21, 2018 at 02:02:57PM +0300, Amir Goldstein wrote: > Yes. If you are looking for a cleanup task, you can > apply relevant patches from my series, starting with: > https://patchwork.kernel.org/patch/9481237/ > (Leave the xfs patch [11/11] out) > > But besides verifying that patches still apply and build, > you will need to address the concerns of fs maintainers. > Take for example the btrfs patch: > https://patchwork.kernel.org/patch/9480725/ > > It says: > + * > + * Values 0..7 should match common file type values in file_type.h. > */ > #define BTRFS_FT_UNKNOWN 0 > #define BTRFS_FT_REG_FILE 1 > > But that is not enough. > When converting code to use the generic defines FT_*, instead of > filesystem defined we need to leave in the code build time assertions > that will catch an attempt to change fs contancts in the future, e.g.: > > static inline u8 btrfs_inode_type(struct inode *inode) > { > - return btrfs_type_by_mode[(inode->i_mode & S_IFMT) >> S_SHIFT]; > + BUILD_BUG_ON(BTRFS_FT_UNKNOWN != FT_UNKNOWN); > + BUILD_BUG_ON(BTRFS_FT_REG_FILE != FT_REG_FILE); > ... > + return fs_umode_to_ftype(inode->i_mode); > } > > Same should be done for all relevant filesystems. > Then you need to hope that fs maintainers will like this cleanup and > want to take the patches ;-) > > Cheers, > Amir. Dear Amir, I will give it a go and see how far I get :-) Regards, Phil