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