* Greg Kurz (gr...@kaod.org) wrote:
> 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>
Queued
> ---
> 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 1553d2ef454f..6592f96f685e 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.26.3
>
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
