[Virtio-fs] [PATCH v3 1/2] virtiofs: passthrough_ll: cleanup getxattr/listxattr
Vivek Goyal
vgoyal at redhat.com
Fri Feb 21 15:49:23 UTC 2020
On Thu, Feb 20, 2020 at 08:47:03PM +0900, Misono Tomohiro wrote:
> This is a cleanup patch to simplify the following xattr fix and
> there is no functional changes.
>
> - Move memory allocation to head of the function
> - Unify fgetxattr/flistxattr call for both size == 0 and
> size != 0 case
> - Remove redundant lo_inode_put call in error path
> (Note: second call is ignored now since @inode is already NULL)
>
> Signed-off-by: Misono Tomohiro <misono.tomohiro at jp.fujitsu.com>
> ---
> tools/virtiofsd/passthrough_ll.c | 62 ++++++++++++++------------------
> 1 file changed, 26 insertions(+), 36 deletions(-)
>
> diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
> index 9772823066..33cff8c2c8 100644
> --- a/tools/virtiofsd/passthrough_ll.c
> +++ b/tools/virtiofsd/passthrough_ll.c
> @@ -2370,8 +2370,8 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *name,
> return;
> }
>
> - saverr = ENOSYS;
> if (!lo_data(req)->xattr) {
> + saverr = ENOSYS;
What's the advange of moving saverr inside bracket. I thougt this is
not needed.
> goto out;
> }
>
> @@ -2384,34 +2384,30 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *name,
> goto out;
> }
>
> + if (size) {
> + value = malloc(size);
> + if (!value) {
> + goto out_err;
> + }
> + }
> +
> sprintf(procname, "%i", inode->fd);
> fd = openat(lo->proc_self_fd, procname, O_RDONLY);
> if (fd < 0) {
> goto out_err;
> }
>
> + ret = fgetxattr(fd, name, value, size);
> + if (ret == -1) {
> + goto out_err;
> + }
> if (size) {
> - value = malloc(size);
> - if (!value) {
> - goto out_err;
> - }
> -
> - ret = fgetxattr(fd, name, value, size);
> - if (ret == -1) {
> - goto out_err;
> - }
> - saverr = 0;
> if (ret == 0) {
> + saverr = 0;
Same here. Why to move saverr inside the brackets.
Have same question for lo_listxattr() modifications.
Thanks
Vivek
> goto out;
> }
> -
> fuse_reply_buf(req, value, ret);
> } else {
> - ret = fgetxattr(fd, name, NULL, 0);
> - if (ret == -1) {
> - goto out_err;
> - }
> -
> fuse_reply_xattr(req, ret);
> }
> out_free:
> @@ -2427,7 +2423,6 @@ out_free:
> out_err:
> saverr = errno;
> out:
> - lo_inode_put(lo, &inode);
> fuse_reply_err(req, saverr);
> goto out_free;
> }
> @@ -2448,8 +2443,8 @@ static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size)
> return;
> }
>
> - saverr = ENOSYS;
> if (!lo_data(req)->xattr) {
> + saverr = ENOSYS;
> goto out;
> }
>
> @@ -2462,34 +2457,30 @@ static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size)
> goto out;
> }
>
> + if (size) {
> + value = malloc(size);
> + if (!value) {
> + goto out_err;
> + }
> + }
> +
> sprintf(procname, "%i", inode->fd);
> fd = openat(lo->proc_self_fd, procname, O_RDONLY);
> if (fd < 0) {
> goto out_err;
> }
>
> + ret = flistxattr(fd, value, size);
> + if (ret == -1) {
> + goto out_err;
> + }
> if (size) {
> - value = malloc(size);
> - if (!value) {
> - goto out_err;
> - }
> -
> - ret = flistxattr(fd, value, size);
> - if (ret == -1) {
> - goto out_err;
> - }
> - saverr = 0;
> if (ret == 0) {
> + saverr = 0;
> goto out;
> }
> -
> fuse_reply_buf(req, value, ret);
> } else {
> - ret = flistxattr(fd, NULL, 0);
> - if (ret == -1) {
> - goto out_err;
> - }
> -
> fuse_reply_xattr(req, ret);
> }
> out_free:
> @@ -2505,7 +2496,6 @@ out_free:
> out_err:
> saverr = errno;
> out:
> - lo_inode_put(lo, &inode);
> fuse_reply_err(req, saverr);
> goto out_free;
> }
> --
> 2.21.1
>
More information about the Virtio-fs
mailing list