<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> </head> <body> <div dir="auto" style="direction: ltr; margin: 0; padding: 0; font-family: sans-serif; font-size: 11pt; color: black; "> Very nice catch David... Countless times I've gotten bit by this one! </div> <div dir="auto" style="direction: ltr; margin: 0; padding: 0; font-family: sans-serif; font-size: 11pt; color: black; "> <div dir="auto" style="direction: ltr; margin: 0; padding: 0; font-family: sans-serif; font-size: 11pt; color: black; "> Get <a href="https://aka.ms/AAb9ysg">Outlook for Android</a></div> </div> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr">From: virtio-fs-bounces@redhat.com <virtio-fs-bounces@redhat.com> on behalf of Dr. David Alan Gilbert (git) <dgilbert@redhat.com> Sent: Thursday, May 6, 2021 1:56:30 PM To: qemu-devel@nongnu.org <qemu-devel@nongnu.org>; groug@kaod.org <groug@kaod.org>; jose.carlos.venegas.munoz@intel.com <jose.carlos.venegas.munoz@intel.com>; ma.mandourr@gmail.com <ma.mandourr@gmail.com> Cc: virtio-fs@redhat.com <virtio-fs@redhat.com>; vgoyal@redhat.com <vgoyal@redhat.com> Subject: [Virtio-fs] [PULL 01/12] virtiofsd: Fix side-effect in assert() <div> </div> </div> <div class="BodyFragment"> <div class="PlainText">External email: Use caution opening links or attachments 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 _______________________________________________ Virtio-fs mailing list Virtio-fs@redhat.com <a href="https://listman.redhat.com/mailman/listinfo/virtio-fs">https://listman.redhat.com/mailman/listinfo/virtio-fs</a> </div> </div> </body> </html>