On May 1, 2014, at 9:48 AM, Aneesh Kumar K.V <aneesh.kumar@xxxxxxxxxxxxxxxxxx> wrote: > Andreas Dilger <adilger@xxxxxxxxx> writes: > >> On Apr 27, 2014, at 10:14 AM, Aneesh Kumar K.V <aneesh.kumar@xxxxxxxxxxxxxxxxxx> wrote: >>> This feature flag can be used to enable richacl on >>> the file system. Once enabled the "acl" mount option >>> will enable richacl instead of posix acl >> >>> diff --git a/fs/ext4/super.c b/fs/ext4/super.c >>> index 6f9e6fadac04..2a0221652d79 100644 >>> --- a/fs/ext4/super.c >>> +++ b/fs/ext4/super.c >>> @@ -1274,6 +1274,30 @@ static ext4_fsblk_t get_sb_block(void **data) >>> return sb_block; >>> } >>> >>> +static void enable_acl(struct super_block *sb) >>> +{ >>> +#if !defined(CONFIG_EXT4_FS_POSIX_ACL) && !defined(CONFIG_EXT4_FS_RICHACL) >>> + return; >>> +#endif >>> + if (EXT4_HAS_COMPAT_FEATURE(sb, EXT4_FEATURE_COMPAT_RICHACL)) { >>> + sb->s_flags |= MS_RICHACL; >>> + sb->s_flags &= ~MS_POSIXACL; >>> + } else { >>> + sb->s_flags |= MS_POSIXACL; >>> + sb->s_flags &= ~MS_RICHACL; >>> + } >> >> This should put the #ifdef around the code that is being enabled/disabled, >> otherwise it just becomes dead code: >> >> static int enable_acl(struct super_block *sb) >> { >> if (EXT4_HAS_COMPAT_FEATURE(sb, EXT4_FEATURE_COMPAT_RICHACL)) { >> #if defined(CONFIG_EXT4_FS_RICHACL) >> sb->s_flags |= MS_RICHACL; >> sb->s_flags &= ~MS_POSIXACL; >> #else >> return -EOPNOTSUPP; >> #endif >> } else { >> #if defined(CONFIG_EXT4_FS_POSIX_ACL) >> sb->s_flags |= MS_POSIXACL; >> sb->s_flags &= ~MS_RICHACL; >> #else >> return -EOPNOTSUPP; >> #endif >> } >> return 0; >> } > > That is too much #ifdef with no real benefit ? The benefit is that if neither CONFIG_EXT4_FS_RICHACL nor CONFIG_EXT4_FS_POSIX_ACL are defined there isn't unreachable code after "return" at the start of the function. Some static code analysis tools will complain about this. Cheers, Andreas
Attachment:
signature.asc
Description: Message signed with OpenPGP using GPGMail