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
