From: "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com>
To: qemu-devel@nongnu.org, groug@kaod.org,
jose.carlos.venegas.munoz@intel.com, ma.mandourr@gmail.com
Cc: virtio-fs@redhat.com, vgoyal@redhat.com, stefanha@redhat.com
Subject: [PULL 01/12] virtiofsd: Fix side-effect in assert()
Date: Thu, 6 May 2021 19:56:30 +0100 [thread overview]
Message-ID: <20210506185641.284821-2-dgilbert@redhat.com> (raw)
In-Reply-To: <20210506185641.284821-1-dgilbert@redhat.com>
From: Greg Kurz <groug@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.tomohiro@jp.fujitsu.com
Signed-off-by: Greg Kurz <groug@kaod.org>
Message-Id: <20210409100627.451573-1-groug@kaod.org>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@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
next prev parent reply other threads:[~2021-05-06 18:59 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-06 18:56 [PULL 00/12] virtiofs queue Dr. David Alan Gilbert (git)
2021-05-06 18:56 ` Dr. David Alan Gilbert (git) [this message]
2021-05-06 19:11 ` [Virtio-fs] [PULL 01/12] virtiofsd: Fix side-effect in assert() Edward McClanahan
2021-05-06 18:56 ` [PULL 02/12] virtiofsd: Allow use "-o xattrmap" without "-o xattr" Dr. David Alan Gilbert (git)
2021-05-06 18:56 ` [PULL 03/12] virtiofsd: Add help for -o xattr-mapping Dr. David Alan Gilbert (git)
2021-05-06 18:56 ` [PULL 04/12] virtiofs: Fixup printf args Dr. David Alan Gilbert (git)
2021-05-06 18:56 ` [PULL 05/12] virtiofsd: Don't assume header layout Dr. David Alan Gilbert (git)
2021-05-06 18:56 ` [PULL 06/12] virtiofsd: Changed allocations of fuse_req to GLib functions Dr. David Alan Gilbert (git)
2021-05-06 18:56 ` [PULL 07/12] virtiofsd: Changed allocations of iovec to GLib's functions Dr. David Alan Gilbert (git)
2021-05-06 18:56 ` [PULL 08/12] virtiofsd: Changed allocations of fuse_session " Dr. David Alan Gilbert (git)
2021-05-06 18:56 ` [PULL 09/12] virtiofsd: Changed allocation of lo_map_elems " Dr. David Alan Gilbert (git)
2021-05-06 18:56 ` [PULL 10/12] virtiofsd: Changed allocations of fv_VuDev & its internals to GLib functions Dr. David Alan Gilbert (git)
2021-05-06 18:56 ` [PULL 11/12] virtiofsd/passthrough_ll.c: Changed local allocations " Dr. David Alan Gilbert (git)
2021-05-06 18:56 ` [PULL 12/12] virtiofsd/fuse_virtio.c: Changed allocations of locals to GLib Dr. David Alan Gilbert (git)
2021-05-11 15:05 ` [PULL 00/12] virtiofs queue Peter Maydell
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210506185641.284821-2-dgilbert@redhat.com \
--to=dgilbert@redhat.com \
--cc=groug@kaod.org \
--cc=jose.carlos.venegas.munoz@intel.com \
--cc=ma.mandourr@gmail.com \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
--cc=vgoyal@redhat.com \
--cc=virtio-fs@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).