On 5/27/20 10:16 AM, Gao Xiang wrote:
On Wed, May 27, 2020 at 09:55:17AM +0800, Chao Yu wrote:

...


Originally, we set erofs_listxattr to ->listxattr only
when the config macro CONFIG_EROFS_FS_XATTR is enabled,
it means that erofs_listxattr() never returns -EOPNOTSUPP
in any case, so actually there is no logic change here,
right?

Yeah, I agree there is no logic change, so I'm fine with the patch.
But I'm little worry about if return 0 is actually wrong here...

see the return value at:
http://man7.org/linux/man-pages/man2/listxattr.2.html

Yeah, I guess vfs should check that whether lower filesystem has set .listxattr
callback function to decide to return that value, something like:

static ssize_t
ecryptfs_listxattr(struct dentry *dentry, char *list, size_t size)
{
...
        if (!d_inode(lower_dentry)->i_op->listxattr) {
                rc = -EOPNOTSUPP;
                goto out;
        }
...
        rc = d_inode(lower_dentry)->i_op->listxattr(lower_dentry, list, size);
...
}

This approach seems better. ;) Let me recheck all of this.
Maybe we could submit a patch to fsdevel for some further advice...


I agree that doing the check in vfs layer looks tidy and unified.
However, we should be aware this change may break user space tools.


Thanks,
cgxu

Reply via email to