Very nice catch David... Countless times I've gotten bit by this one!
Get Outlook for Android<https://aka.ms/AAb9ysg>
________________________________
From: virtio-fs-boun...@redhat.com <virtio-fs-boun...@redhat.com> on behalf of
Dr. David Alan Gilbert (git) <dgilb...@redhat.com>
Sent: Thursday, May 6, 2021 1:56:30 PM
To: qemu-devel@nongnu.org <qemu-devel@nongnu.org>; gr...@kaod.org
<gr...@kaod.org>; jose.carlos.venegas.mu...@intel.com
<jose.carlos.venegas.mu...@intel.com>; ma.mando...@gmail.com
<ma.mando...@gmail.com>
Cc: virtio...@redhat.com <virtio...@redhat.com>; vgo...@redhat.com
<vgo...@redhat.com>
Subject: [Virtio-fs] [PULL 01/12] virtiofsd: Fix side-effect in assert()
External email: Use caution opening links or attachments
From: Greg Kurz <gr...@kaod.org>
It is bad practice to put an expression with a side-effect in
assert() because the side-effect won't happen if the code is
compiled with -DNDEBUG.
Use an intermediate variable. Consolidate this in an macro to
have proper line numbers when the assertion is hit.
virtiofsd: ../../tools/virtiofsd/passthrough_ll.c:2797: lo_getxattr:
Assertion `fchdir_res == 0' failed.
Aborted
2796 /* fchdir should not fail here */
=>2797 FCHDIR_NOFAIL(lo->proc_self_fd);
2798 ret = getxattr(procname, name, value, size);
2799 FCHDIR_NOFAIL(lo->root.fd);
Fixes: bdfd66788349 ("virtiofsd: Fix xattr operations")
Cc: misono.tomoh...@jp.fujitsu.com
Signed-off-by: Greg Kurz <gr...@kaod.org>
Message-Id: <20210409100627.451573-1-gr...@kaod.org>
Signed-off-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com>
---
tools/virtiofsd/passthrough_ll.c | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 1553d2ef45..6592f96f68 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -2723,6 +2723,11 @@ static int xattr_map_server(const struct lo_data *lo,
const char *server_name,
return -ENODATA;
}
+#define FCHDIR_NOFAIL(fd) do { \
+ int fchdir_res = fchdir(fd); \
+ assert(fchdir_res == 0); \
+ } while (0)
+
static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name,
size_t size)
{
@@ -2789,9 +2794,9 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t ino,
const char *in_name,
ret = fgetxattr(fd, name, value, size);
} else {
/* fchdir should not fail here */
- assert(fchdir(lo->proc_self_fd) == 0);
+ FCHDIR_NOFAIL(lo->proc_self_fd);
ret = getxattr(procname, name, value, size);
- assert(fchdir(lo->root.fd) == 0);
+ FCHDIR_NOFAIL(lo->root.fd);
}
if (ret == -1) {
@@ -2864,9 +2869,9 @@ static void lo_listxattr(fuse_req_t req, fuse_ino_t ino,
size_t size)
ret = flistxattr(fd, value, size);
} else {
/* fchdir should not fail here */
- assert(fchdir(lo->proc_self_fd) == 0);
+ FCHDIR_NOFAIL(lo->proc_self_fd);
ret = listxattr(procname, value, size);
- assert(fchdir(lo->root.fd) == 0);
+ FCHDIR_NOFAIL(lo->root.fd);
}
if (ret == -1) {
@@ -3000,9 +3005,9 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino,
const char *in_name,
ret = fsetxattr(fd, name, value, size, flags);
} else {
/* fchdir should not fail here */
- assert(fchdir(lo->proc_self_fd) == 0);
+ FCHDIR_NOFAIL(lo->proc_self_fd);
ret = setxattr(procname, name, value, size, flags);
- assert(fchdir(lo->root.fd) == 0);
+ FCHDIR_NOFAIL(lo->root.fd);
}
saverr = ret == -1 ? errno : 0;
@@ -3066,9 +3071,9 @@ static void lo_removexattr(fuse_req_t req, fuse_ino_t
ino, const char *in_name)
ret = fremovexattr(fd, name);
} else {
/* fchdir should not fail here */
- assert(fchdir(lo->proc_self_fd) == 0);
+ FCHDIR_NOFAIL(lo->proc_self_fd);
ret = removexattr(procname, name);
- assert(fchdir(lo->root.fd) == 0);
+ FCHDIR_NOFAIL(lo->root.fd);
}
saverr = ret == -1 ? errno : 0;
--
2.31.1
_______________________________________________
Virtio-fs mailing list
virtio...@redhat.com
https://listman.redhat.com/mailman/listinfo/virtio-fs
