Re: [PATCH 1/2] fs/ntfs3: Remove unnecessary functions

[Konstantin Komarov]

On 16.10.2021 13:42, Kari Argillander wrote:
> On Fri, Oct 01, 2021 at 07:02:12PM +0300, Konstantin Komarov wrote:
>> We don't need ntfs_xattr_get_acl and ntfs_xattr_set_acl.
>> There are ntfs_get_acl_ex and ntfs_set_acl_ex.
> 
> I just bisect this commit after tests
> 
> "generic/099,generic/105,generic/307,generic/318,generic/319,generic/375,generic/444"
> 
> fails for me. Fails happends because mount option acl was not defined.
> Before they where skipped, but now fail occurs. Also generic/099 was
> passing if acl mount option was defined, but after this patch it also
> fail. Every other test fail for me as well, but that is not related to
> this patch.
> 
> So should we revert or do you make new patch to fix the issue or do you
> think we won't have any issue here?
> 
>   Argillander
> 

We skipped these tests(muted them in beginning and forgot to unmute).
Thanks for pointing this out. There will be patch to fix them.

>> Signed-off-by: Konstantin Komarov <almaz.alexandrovich@xxxxxxxxxxxxxxxxxxxx>
>> ---
>>  fs/ntfs3/xattr.c | 94 ------------------------------------------------
>>  1 file changed, 94 deletions(-)
>>
>> diff --git a/fs/ntfs3/xattr.c b/fs/ntfs3/xattr.c
>> index 83bbee277e12..111355692163 100644
>> --- a/fs/ntfs3/xattr.c
>> +++ b/fs/ntfs3/xattr.c
>> @@ -621,67 +621,6 @@ int ntfs_set_acl(struct user_namespace *mnt_userns, struct inode *inode,
>>  	return ntfs_set_acl_ex(mnt_userns, inode, acl, type, 0);
>>  }
>>  
>> -static int ntfs_xattr_get_acl(struct user_namespace *mnt_userns,
>> -			      struct inode *inode, int type, void *buffer,
>> -			      size_t size)
>> -{
>> -	struct posix_acl *acl;
>> -	int err;
>> -
>> -	if (!(inode->i_sb->s_flags & SB_POSIXACL)) {
>> -		ntfs_inode_warn(inode, "add mount option \"acl\" to use acl");
>> -		return -EOPNOTSUPP;
>> -	}
>> -
>> -	acl = ntfs_get_acl(inode, type);
>> -	if (IS_ERR(acl))
>> -		return PTR_ERR(acl);
>> -
>> -	if (!acl)
>> -		return -ENODATA;
>> -
>> -	err = posix_acl_to_xattr(mnt_userns, acl, buffer, size);
>> -	ntfs_posix_acl_release(acl);
>> -
>> -	return err;
>> -}
>> -
>> -static int ntfs_xattr_set_acl(struct user_namespace *mnt_userns,
>> -			      struct inode *inode, int type, const void *value,
>> -			      size_t size)
>> -{
>> -	struct posix_acl *acl;
>> -	int err;
>> -
>> -	if (!(inode->i_sb->s_flags & SB_POSIXACL)) {
>> -		ntfs_inode_warn(inode, "add mount option \"acl\" to use acl");
>> -		return -EOPNOTSUPP;
>> -	}
>> -
>> -	if (!inode_owner_or_capable(mnt_userns, inode))
>> -		return -EPERM;
>> -
>> -	if (!value) {
>> -		acl = NULL;
>> -	} else {
>> -		acl = posix_acl_from_xattr(mnt_userns, value, size);
>> -		if (IS_ERR(acl))
>> -			return PTR_ERR(acl);
>> -
>> -		if (acl) {
>> -			err = posix_acl_valid(mnt_userns, acl);
>> -			if (err)
>> -				goto release_and_out;
>> -		}
>> -	}
>> -
>> -	err = ntfs_set_acl(mnt_userns, inode, acl, type);
>> -
>> -release_and_out:
>> -	ntfs_posix_acl_release(acl);
>> -	return err;
>> -}
>> -
>>  /*
>>   * ntfs_init_acl - Initialize the ACLs of a new inode.
>>   *
>> @@ -848,23 +787,6 @@ static int ntfs_getxattr(const struct xattr_handler *handler, struct dentry *de,
>>  		goto out;
>>  	}
>>  
>> -#ifdef CONFIG_NTFS3_FS_POSIX_ACL
>> -	if ((name_len == sizeof(XATTR_NAME_POSIX_ACL_ACCESS) - 1 &&
>> -	     !memcmp(name, XATTR_NAME_POSIX_ACL_ACCESS,
>> -		     sizeof(XATTR_NAME_POSIX_ACL_ACCESS))) ||
>> -	    (name_len == sizeof(XATTR_NAME_POSIX_ACL_DEFAULT) - 1 &&
>> -	     !memcmp(name, XATTR_NAME_POSIX_ACL_DEFAULT,
>> -		     sizeof(XATTR_NAME_POSIX_ACL_DEFAULT)))) {
>> -		/* TODO: init_user_ns? */
>> -		err = ntfs_xattr_get_acl(
>> -			&init_user_ns, inode,
>> -			name_len == sizeof(XATTR_NAME_POSIX_ACL_ACCESS) - 1
>> -				? ACL_TYPE_ACCESS
>> -				: ACL_TYPE_DEFAULT,
>> -			buffer, size);
>> -		goto out;
>> -	}
>> -#endif
>>  	/* Deal with NTFS extended attribute. */
>>  	err = ntfs_get_ea(inode, name, name_len, buffer, size, NULL);
>>  
>> @@ -977,22 +899,6 @@ static noinline int ntfs_setxattr(const struct xattr_handler *handler,
>>  		goto out;
>>  	}
>>  
>> -#ifdef CONFIG_NTFS3_FS_POSIX_ACL
>> -	if ((name_len == sizeof(XATTR_NAME_POSIX_ACL_ACCESS) - 1 &&
>> -	     !memcmp(name, XATTR_NAME_POSIX_ACL_ACCESS,
>> -		     sizeof(XATTR_NAME_POSIX_ACL_ACCESS))) ||
>> -	    (name_len == sizeof(XATTR_NAME_POSIX_ACL_DEFAULT) - 1 &&
>> -	     !memcmp(name, XATTR_NAME_POSIX_ACL_DEFAULT,
>> -		     sizeof(XATTR_NAME_POSIX_ACL_DEFAULT)))) {
>> -		err = ntfs_xattr_set_acl(
>> -			mnt_userns, inode,
>> -			name_len == sizeof(XATTR_NAME_POSIX_ACL_ACCESS) - 1
>> -				? ACL_TYPE_ACCESS
>> -				: ACL_TYPE_DEFAULT,
>> -			value, size);
>> -		goto out;
>> -	}
>> -#endif
>>  	/* Deal with NTFS extended attribute. */
>>  	err = ntfs_set_ea(inode, name, name_len, value, size, flags, 0);
>>  
>> -- 
>> 2.33.0
>>