From a04ba40349816021a62c8aea264afe04bdb44b67 Mon Sep 17 00:00:00 2001 From: asomers Date: Wed, 26 Jun 2019 20:25:57 +0000 Subject: [PATCH] fusefs: annotate deliberate file descriptor leaks in the tests closing a file descriptor causes FUSE activity that is superfluous to the purpose of most tests, but would nonetheless require matching expectations. Rather than do that, most tests deliberately leak file descriptors instead. This commit moves the leakage from each test into two trivial functions: leak and leakdir. Hopefully Coverity will only complain about those functions and not all of their callers. Sponsored by: The FreeBSD Foundation --- tests/sys/fs/fusefs/allow_other.cc | 3 +- tests/sys/fs/fusefs/create.cc | 18 ++++----- tests/sys/fs/fusefs/default_permissions.cc | 12 +++--- tests/sys/fs/fusefs/fifo.cc | 4 +- tests/sys/fs/fusefs/flush.cc | 3 +- tests/sys/fs/fusefs/fsync.cc | 10 ++--- tests/sys/fs/fusefs/fsyncdir.cc | 10 ++--- tests/sys/fs/fusefs/io.cc | 2 +- tests/sys/fs/fusefs/locks.cc | 26 ++++++------- tests/sys/fs/fusefs/nfs.cc | 2 +- tests/sys/fs/fusefs/notify.cc | 8 ++-- tests/sys/fs/fusefs/open.cc | 2 +- tests/sys/fs/fusefs/read.cc | 45 ++++++++++++---------- tests/sys/fs/fusefs/readdir.cc | 15 ++++---- tests/sys/fs/fusefs/setattr.cc | 4 +- tests/sys/fs/fusefs/unlink.cc | 2 +- tests/sys/fs/fusefs/utils.hh | 21 ++++++++++ tests/sys/fs/fusefs/write.cc | 42 ++++++++++---------- 18 files changed, 128 insertions(+), 101 deletions(-) diff --git a/tests/sys/fs/fusefs/allow_other.cc b/tests/sys/fs/fusefs/allow_other.cc index b05157a3522..61ff5e9b5bf 100644 --- a/tests/sys/fs/fusefs/allow_other.cc +++ b/tests/sys/fs/fusefs/allow_other.cc @@ -182,11 +182,12 @@ TEST_F(AllowOther, privilege_escalation) strerror(errno)); return 1; } + leak(fd0); return 0; } ); ASSERT_EQ(0, WEXITSTATUS(status)); - /* Deliberately leak fd1. close(2) will be tested in release.cc */ + leak(fd1); } TEST_F(NoAllowOther, disallowed) diff --git a/tests/sys/fs/fusefs/create.cc b/tests/sys/fs/fusefs/create.cc index 6d0ce71a99c..1030a1e5633 100644 --- a/tests/sys/fs/fusefs/create.cc +++ b/tests/sys/fs/fusefs/create.cc @@ -142,7 +142,7 @@ TEST_F(Create, attr_cache) fd = open(FULLPATH, O_CREAT | O_EXCL, mode); EXPECT_LE(0, fd) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* A successful CREATE operation should purge the parent dir's attr cache */ @@ -185,7 +185,7 @@ TEST_F(Create, clear_attr_cache) EXPECT_LE(0, fd) << strerror(errno); EXPECT_EQ(0, stat("mountpoint", &sb)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* @@ -253,7 +253,7 @@ TEST_F(Create, Enosys) fd = open(FULLPATH, O_CREAT | O_EXCL, mode); EXPECT_LE(0, fd) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* @@ -287,7 +287,7 @@ TEST_F(Create, entry_cache_negative) fd = open(FULLPATH, O_CREAT | O_EXCL, mode); ASSERT_LE(0, fd) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* @@ -323,7 +323,7 @@ TEST_F(Create, entry_cache_negative_purge) expect_lookup(RELPATH, ino, S_IFREG | mode, 0, 1); ASSERT_EQ(0, access(FULLPATH, F_OK)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* @@ -365,7 +365,7 @@ TEST_F(Create, ok) fd = open(FULLPATH, O_CREAT | O_EXCL, mode); EXPECT_LE(0, fd) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* @@ -397,7 +397,7 @@ TEST_F(Create, wronly_0444) fd = open(FULLPATH, O_CREAT | O_WRONLY, mode); EXPECT_LE(0, fd) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } TEST_F(Create_7_8, ok) @@ -421,7 +421,7 @@ TEST_F(Create_7_8, ok) fd = open(FULLPATH, O_CREAT | O_EXCL, mode); EXPECT_LE(0, fd) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } TEST_F(Create_7_11, ok) @@ -445,5 +445,5 @@ TEST_F(Create_7_11, ok) fd = open(FULLPATH, O_CREAT | O_EXCL, mode); EXPECT_LE(0, fd) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } diff --git a/tests/sys/fs/fusefs/default_permissions.cc b/tests/sys/fs/fusefs/default_permissions.cc index a416f13965e..41ba7022d33 100644 --- a/tests/sys/fs/fusefs/default_permissions.cc +++ b/tests/sys/fs/fusefs/default_permissions.cc @@ -488,7 +488,7 @@ TEST_F(Create, ok) fd = open(FULLPATH, O_CREAT | O_EXCL, 0644); EXPECT_LE(0, fd) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } TEST_F(Create, eacces) @@ -763,7 +763,7 @@ TEST_F(Open, ok) fd = open(FULLPATH, O_RDONLY); EXPECT_LE(0, fd) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } TEST_F(Rename, eacces_on_srcdir) @@ -1024,7 +1024,7 @@ TEST_F(Setattr, ftruncate_of_newly_created_file) fd = open(FULLPATH, O_CREAT | O_RDWR, 0); ASSERT_LE(0, fd) << strerror(errno); ASSERT_EQ(0, ftruncate(fd, 100)) << strerror(errno); - /* Deliberately leak fd */ + leak(fd); } /* @@ -1242,7 +1242,7 @@ TEST_F(Write, clear_suid) ASSERT_EQ(1, write(fd, wbuf, sizeof(wbuf))) << strerror(errno); ASSERT_EQ(0, fstat(fd, &sb)) << strerror(errno); EXPECT_EQ(S_IFREG | newmode, sb.st_mode); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* A write by a non-owner should clear a file's SGID bit */ @@ -1268,7 +1268,7 @@ TEST_F(Write, clear_sgid) ASSERT_EQ(1, write(fd, wbuf, sizeof(wbuf))) << strerror(errno); ASSERT_EQ(0, fstat(fd, &sb)) << strerror(errno); EXPECT_EQ(S_IFREG | newmode, sb.st_mode); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* Regression test for a specific recurse-of-nonrecursive-lock panic @@ -1297,7 +1297,7 @@ TEST_F(Write, recursion_panic_while_clearing_suid) fd = open(FULLPATH, O_WRONLY); ASSERT_LE(0, fd) << strerror(errno); ASSERT_EQ(1, write(fd, wbuf, sizeof(wbuf))) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } diff --git a/tests/sys/fs/fusefs/fifo.cc b/tests/sys/fs/fusefs/fifo.cc index 8a6037a92d2..be443a1ef0a 100644 --- a/tests/sys/fs/fusefs/fifo.cc +++ b/tests/sys/fs/fusefs/fifo.cc @@ -106,7 +106,7 @@ TEST_F(Fifo, read_write) } ASSERT_STREQ(message, MESSAGE); - /* Deliberately leak fd */ + leak(fd); } /* Writer thread */ @@ -203,5 +203,5 @@ TEST_F(Socket, read_write) } ASSERT_STREQ(message, MESSAGE); - /* Deliberately leak fd */ + leak(fd); } diff --git a/tests/sys/fs/fusefs/flush.cc b/tests/sys/fs/fusefs/flush.cc index c501984ebaa..01aca311ffc 100644 --- a/tests/sys/fs/fusefs/flush.cc +++ b/tests/sys/fs/fusefs/flush.cc @@ -227,5 +227,6 @@ TEST_F(FlushWithLocks, unlock_on_close) fd2 = open(FULLPATH, O_WRONLY); ASSERT_LE(0, fd2) << strerror(errno); ASSERT_EQ(0, close(fd2)) << strerror(errno); - /* Deliberately leak fd */ + leak(fd); + leak(fd2); } diff --git a/tests/sys/fs/fusefs/fsync.cc b/tests/sys/fs/fusefs/fsync.cc index 776ae29883a..b283f065c1f 100644 --- a/tests/sys/fs/fusefs/fsync.cc +++ b/tests/sys/fs/fusefs/fsync.cc @@ -106,7 +106,7 @@ TEST_F(Fsync, aio_fsync) ASSERT_EQ(0, aio_fsync(O_SYNC, &iocb)) << strerror(errno); ASSERT_EQ(0, aio_waitcomplete(&piocb, NULL)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* @@ -171,7 +171,7 @@ TEST_F(Fsync, eio) ASSERT_NE(0, fdatasync(fd)); ASSERT_EQ(EIO, errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* @@ -200,7 +200,7 @@ TEST_F(Fsync, enosys) /* Subsequent calls shouldn't query the daemon*/ EXPECT_EQ(0, fdatasync(fd)); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } @@ -223,7 +223,7 @@ TEST_F(Fsync, fdatasync) ASSERT_EQ(bufsize, write(fd, CONTENTS, bufsize)) << strerror(errno); ASSERT_EQ(0, fdatasync(fd)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } TEST_F(Fsync, fsync) @@ -245,5 +245,5 @@ TEST_F(Fsync, fsync) ASSERT_EQ(bufsize, write(fd, CONTENTS, bufsize)) << strerror(errno); ASSERT_EQ(0, fsync(fd)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } diff --git a/tests/sys/fs/fusefs/fsyncdir.cc b/tests/sys/fs/fusefs/fsyncdir.cc index c7cc4c72131..b87f8c47a2e 100644 --- a/tests/sys/fs/fusefs/fsyncdir.cc +++ b/tests/sys/fs/fusefs/fsyncdir.cc @@ -97,7 +97,7 @@ TEST_F(FsyncDir, aio_fsync) ASSERT_EQ(0, aio_fsync(O_SYNC, &iocb)) << strerror(errno); ASSERT_EQ(0, aio_waitcomplete(&piocb, NULL)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } TEST_F(FsyncDir, eio) @@ -116,7 +116,7 @@ TEST_F(FsyncDir, eio) ASSERT_NE(0, fsync(fd)); ASSERT_EQ(EIO, errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* @@ -142,7 +142,7 @@ TEST_F(FsyncDir, enosys) /* Subsequent calls shouldn't query the daemon*/ EXPECT_EQ(0, fsync(fd)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } TEST_F(FsyncDir, fsyncdata) @@ -160,7 +160,7 @@ TEST_F(FsyncDir, fsyncdata) ASSERT_LE(0, fd) << strerror(errno); ASSERT_EQ(0, fdatasync(fd)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* @@ -182,5 +182,5 @@ TEST_F(FsyncDir, fsync) ASSERT_LE(0, fd) << strerror(errno); ASSERT_EQ(0, fsync(fd)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } diff --git a/tests/sys/fs/fusefs/io.cc b/tests/sys/fs/fusefs/io.cc index d12e43784f7..12e2c67f2bd 100644 --- a/tests/sys/fs/fusefs/io.cc +++ b/tests/sys/fs/fusefs/io.cc @@ -230,7 +230,7 @@ void TearDown() if (m_control_fd >= 0) close(m_control_fd); FuseTest::TearDown(); - /* Deliberately leak test_fd */ + leak(m_test_fd); } void do_closeopen() diff --git a/tests/sys/fs/fusefs/locks.cc b/tests/sys/fs/fusefs/locks.cc index dc6c3e41770..1d328d9f92d 100644 --- a/tests/sys/fs/fusefs/locks.cc +++ b/tests/sys/fs/fusefs/locks.cc @@ -127,7 +127,7 @@ TEST_F(FlockFallback, local) fd = open(FULLPATH, O_RDWR); ASSERT_LE(0, fd) << strerror(errno); ASSERT_EQ(0, flock(fd, LOCK_EX)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* @@ -148,7 +148,7 @@ TEST_F(Flock, local) fd = open(FULLPATH, O_RDWR); ASSERT_LE(0, fd) << strerror(errno); ASSERT_EQ(0, flock(fd, LOCK_EX)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* Set a new flock lock with FUSE_SETLK */ @@ -167,7 +167,7 @@ TEST_F(Flock, DISABLED_set) fd = open(FULLPATH, O_RDWR); ASSERT_LE(0, fd) << strerror(errno); ASSERT_EQ(0, flock(fd, LOCK_EX)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* Fail to set a flock lock in non-blocking mode */ @@ -187,7 +187,7 @@ TEST_F(Flock, DISABLED_eagain) ASSERT_LE(0, fd) << strerror(errno); ASSERT_NE(0, flock(fd, LOCK_EX | LOCK_NB)); ASSERT_EQ(EAGAIN, errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* @@ -214,7 +214,7 @@ TEST_F(GetlkFallback, local) fl.l_whence = SEEK_SET; fl.l_sysid = 0; ASSERT_NE(-1, fcntl(fd, F_GETLK, &fl)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* @@ -260,7 +260,7 @@ TEST_F(Getlk, no_locks) fl.l_sysid = 0; ASSERT_NE(-1, fcntl(fd, F_GETLK, &fl)) << strerror(errno); ASSERT_EQ(F_UNLCK, fl.l_type); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* A different pid does have a lock */ @@ -311,7 +311,7 @@ TEST_F(Getlk, lock_exists) EXPECT_EQ(F_WRLCK, fl.l_type); EXPECT_EQ(SEEK_SET, fl.l_whence); EXPECT_EQ(0, fl.l_sysid); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* @@ -338,7 +338,7 @@ TEST_F(SetlkFallback, local) fl.l_whence = SEEK_SET; fl.l_sysid = 0; ASSERT_NE(-1, fcntl(fd, F_SETLK, &fl)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* Set a new lock with FUSE_SETLK */ @@ -364,7 +364,7 @@ TEST_F(Setlk, set) fl.l_whence = SEEK_SET; fl.l_sysid = 0; ASSERT_NE(-1, fcntl(fd, F_SETLK, &fl)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* l_len = 0 is a flag value that means to lock until EOF */ @@ -390,7 +390,7 @@ TEST_F(Setlk, set_eof) fl.l_whence = SEEK_SET; fl.l_sysid = 0; ASSERT_NE(-1, fcntl(fd, F_SETLK, &fl)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* Fail to set a new lock with FUSE_SETLK due to a conflict */ @@ -417,7 +417,7 @@ TEST_F(Setlk, eagain) fl.l_sysid = 0; ASSERT_EQ(-1, fcntl(fd, F_SETLK, &fl)); ASSERT_EQ(EAGAIN, errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* @@ -444,7 +444,7 @@ TEST_F(SetlkwFallback, local) fl.l_whence = SEEK_SET; fl.l_sysid = 0; ASSERT_NE(-1, fcntl(fd, F_SETLKW, &fl)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* @@ -474,5 +474,5 @@ TEST_F(Setlkw, set) fl.l_whence = SEEK_SET; fl.l_sysid = 0; ASSERT_NE(-1, fcntl(fd, F_SETLKW, &fl)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } diff --git a/tests/sys/fs/fusefs/nfs.cc b/tests/sys/fs/fusefs/nfs.cc index ef2d0310106..b33df0af50d 100644 --- a/tests/sys/fs/fusefs/nfs.cc +++ b/tests/sys/fs/fusefs/nfs.cc @@ -340,5 +340,5 @@ TEST_F(Readdir, getdirentries) r = getdirentries(fd, buf, sizeof(buf), 0); ASSERT_EQ(0, r) << strerror(errno); - /* Deliberately leak fd. RELEASEDIR will be tested separately */ + leak(fd); } diff --git a/tests/sys/fs/fusefs/notify.cc b/tests/sys/fs/fusefs/notify.cc index 957284048a4..b6e29a7d429 100644 --- a/tests/sys/fs/fusefs/notify.cc +++ b/tests/sys/fs/fusefs/notify.cc @@ -383,7 +383,7 @@ TEST_F(Notify, inval_inode_with_clean_cache) ASSERT_EQ(size1, read(fd, buf, size1)) << strerror(errno); EXPECT_EQ(0, memcmp(buf, CONTENTS1, size1)); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* FUSE_NOTIFY_STORE with a file that's not in the entry cache */ @@ -442,7 +442,7 @@ TEST_F(Notify, DISABLED_store_with_blank_cache) ASSERT_EQ(size1, read(fd, buf, size1)) << strerror(errno); EXPECT_EQ(0, memcmp(buf, CONTENTS1, size1)); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } TEST_F(NotifyWriteback, inval_inode_with_dirty_cache) @@ -482,7 +482,7 @@ TEST_F(NotifyWriteback, inval_inode_with_dirty_cache) pthread_join(th0, &thr0_value); EXPECT_EQ(0, (intptr_t)thr0_value); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } TEST_F(NotifyWriteback, inval_inode_attrs_only) @@ -541,5 +541,5 @@ TEST_F(NotifyWriteback, inval_inode_attrs_only) EXPECT_EQ(uid, sb.st_uid); EXPECT_EQ(bufsize, sb.st_size); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } diff --git a/tests/sys/fs/fusefs/open.cc b/tests/sys/fs/fusefs/open.cc index 7886097eaaf..20d1421ce7f 100644 --- a/tests/sys/fs/fusefs/open.cc +++ b/tests/sys/fs/fusefs/open.cc @@ -64,7 +64,7 @@ void test_ok(int os_flags, int fuse_flags) { fd = open(FULLPATH, os_flags); EXPECT_LE(0, fd) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } }; diff --git a/tests/sys/fs/fusefs/read.cc b/tests/sys/fs/fusefs/read.cc index ec543c0d649..fa70949ab60 100644 --- a/tests/sys/fs/fusefs/read.cc +++ b/tests/sys/fs/fusefs/read.cc @@ -136,7 +136,8 @@ TEST_F(AioRead, aio_read) ASSERT_EQ(0, aio_read(&iocb)) << strerror(errno); ASSERT_EQ(bufsize, aio_waitcomplete(&piocb, NULL)) << strerror(errno); ASSERT_EQ(0, memcmp(buf, CONTENTS, bufsize)); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + + leak(fd); } /* @@ -216,7 +217,7 @@ TEST_F(AioRead, async_read_disabled) /* Wait for AIO activity to complete, but ignore errors */ (void)aio_waitcomplete(NULL, NULL); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* @@ -295,7 +296,7 @@ TEST_F(AsyncRead, async_read) /* Wait for AIO activity to complete, but ignore errors */ (void)aio_waitcomplete(NULL, NULL); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* 0-length reads shouldn't cause any confusion */ @@ -315,7 +316,7 @@ TEST_F(Read, direct_io_read_nothing) ASSERT_LE(0, fd) << strerror(errno); ASSERT_EQ(0, pread(fd, buf, 0, offset)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* @@ -348,7 +349,7 @@ TEST_F(Read, direct_io_pread) expect_read(ino, offset, bufsize, bufsize, CONTENTS); ASSERT_EQ(bufsize, pread(fd, buf, bufsize, offset)) << strerror(errno); ASSERT_EQ(0, memcmp(buf, CONTENTS, bufsize)); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* @@ -377,7 +378,7 @@ TEST_F(Read, direct_io_short_read) ASSERT_EQ(halfbufsize, pread(fd, buf, bufsize, offset)) << strerror(errno); ASSERT_EQ(0, memcmp(buf, CONTENTS, halfbufsize)); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } TEST_F(Read, eio) @@ -404,7 +405,7 @@ TEST_F(Read, eio) ASSERT_EQ(-1, read(fd, buf, bufsize)) << strerror(errno); ASSERT_EQ(EIO, errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* @@ -439,7 +440,7 @@ TEST_F(Read, eof) EXPECT_EQ(partbufsize, r) << strerror(errno); ASSERT_EQ(0, fstat(fd, &sb)); EXPECT_EQ((off_t)(offset + partbufsize), sb.st_size); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* Like Read.eof, but causes an entire buffer to be invalidated */ @@ -472,7 +473,7 @@ TEST_F(Read, eof_of_whole_buffer) << strerror(errno); ASSERT_EQ(0, fstat(fd, &sb)); EXPECT_EQ((off_t)(m_maxbcachebuf), sb.st_size); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* @@ -506,7 +507,8 @@ TEST_F(Read, keep_cache) */ ASSERT_EQ(bufsize, read(fd1, buf, bufsize)) << strerror(errno); - /* Deliberately leak fd0 and fd1. */ + leak(fd0); + leak(fd1); } /* @@ -542,7 +544,8 @@ TEST_F(Read, keep_cache_disabled) ASSERT_EQ(0, lseek(fd0, 0, SEEK_SET)) << strerror(errno); ASSERT_EQ(bufsize, read(fd0, buf, bufsize)) << strerror(errno); - /* Deliberately leak fd0 and fd1. */ + leak(fd0); + leak(fd1); } TEST_F(Read, mmap) @@ -584,7 +587,7 @@ TEST_F(Read, mmap) ASSERT_EQ(0, memcmp(p, CONTENTS, bufsize)); ASSERT_EQ(0, munmap(p, len)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* @@ -635,7 +638,7 @@ TEST_F(Read, mmap_eof) EXPECT_EQ((off_t)bufsize, sb.st_size); ASSERT_EQ(0, munmap(p, len)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* @@ -670,7 +673,7 @@ TEST_F(Read, o_direct) ASSERT_EQ(bufsize, read(fd, buf, bufsize)) << strerror(errno); ASSERT_EQ(0, memcmp(buf, CONTENTS, bufsize)); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } TEST_F(Read, pread) @@ -697,7 +700,7 @@ TEST_F(Read, pread) ASSERT_EQ(bufsize, pread(fd, buf, bufsize, offset)) << strerror(errno); ASSERT_EQ(0, memcmp(buf, CONTENTS, bufsize)); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } TEST_F(Read, read) @@ -720,7 +723,7 @@ TEST_F(Read, read) ASSERT_EQ(bufsize, read(fd, buf, bufsize)) << strerror(errno); ASSERT_EQ(0, memcmp(buf, CONTENTS, bufsize)); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } TEST_F(Read_7_8, read) @@ -743,7 +746,7 @@ TEST_F(Read_7_8, read) ASSERT_EQ(bufsize, read(fd, buf, bufsize)) << strerror(errno); ASSERT_EQ(0, memcmp(buf, CONTENTS, bufsize)); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* @@ -781,7 +784,7 @@ TEST_F(Read, cache_block) /* A subsequent read should be serviced by cache */ ASSERT_EQ(bufsize, read(fd, buf, bufsize)) << strerror(errno); ASSERT_EQ(0, memcmp(buf, contents1, bufsize)); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* Reading with sendfile should work (though it obviously won't be 0-copy) */ @@ -827,7 +830,7 @@ TEST_F(Read, sendfile) close(sp[1]); close(sp[0]); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* sendfile should fail gracefully if fuse declines the read */ @@ -861,7 +864,7 @@ TEST_F(Read, DISABLED_sendfile_eio) close(sp[1]); close(sp[0]); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* @@ -903,7 +906,7 @@ TEST_P(ReadAhead, readahead) { ASSERT_EQ(bufsize, read(fd, rbuf, bufsize)) << strerror(errno); ASSERT_EQ(0, memcmp(rbuf, contents, bufsize)); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } INSTANTIATE_TEST_CASE_P(RA, ReadAhead, diff --git a/tests/sys/fs/fusefs/readdir.cc b/tests/sys/fs/fusefs/readdir.cc index 24117a52fee..81edccb8b2e 100644 --- a/tests/sys/fs/fusefs/readdir.cc +++ b/tests/sys/fs/fusefs/readdir.cc @@ -117,7 +117,7 @@ TEST_F(Readdir, dots) ASSERT_EQ(NULL, readdir(dir)); ASSERT_EQ(0, errno); - /* Deliberately leak dir. RELEASEDIR will be tested separately */ + leakdir(dir); } TEST_F(Readdir, eio) @@ -148,7 +148,7 @@ TEST_F(Readdir, eio) ASSERT_EQ(NULL, de); ASSERT_EQ(EIO, errno); - /* Deliberately leak dir. RELEASEDIR will be tested separately */ + leakdir(dir); } /* getdirentries(2) can use a larger buffer size than readdir(3) */ @@ -181,7 +181,7 @@ TEST_F(Readdir, getdirentries) r = getdirentries(fd, buf, sizeof(buf), 0); ASSERT_EQ(0, r) << strerror(errno); - /* Deliberately leak fd. RELEASEDIR will be tested separately */ + leak(fd); } /* @@ -228,7 +228,8 @@ TEST_F(Readdir, getdirentries_concurrent) r = getdirentries(fd1, buf, sizeof(buf), 0); ASSERT_EQ(0, r) << strerror(errno); - /* Deliberately leak fd1. */ + leak(fd0); + leak(fd1); } /* @@ -263,7 +264,7 @@ TEST_F(Readdir, nodots) ASSERT_EQ(NULL, readdir(dir)); ASSERT_EQ(0, errno); - /* Deliberately leak dir. RELEASEDIR will be tested separately */ + leakdir(dir); } /* telldir(3) and seekdir(3) should work with fuse */ @@ -339,7 +340,7 @@ TEST_F(Readdir, seekdir) ASSERT_NE(NULL, de) << strerror(errno); EXPECT_EQ(130ul, de->d_fileno); - /* Deliberately leak dir. RELEASEDIR will be tested separately */ + leakdir(dir); } TEST_F(Readdir_7_8, nodots) @@ -370,5 +371,5 @@ TEST_F(Readdir_7_8, nodots) ASSERT_EQ(NULL, readdir(dir)); ASSERT_EQ(0, errno); - /* Deliberately leak dir. RELEASEDIR will be tested separately */ + leakdir(dir); } diff --git a/tests/sys/fs/fusefs/setattr.cc b/tests/sys/fs/fusefs/setattr.cc index 0c3bd31c72c..fe82038c6aa 100644 --- a/tests/sys/fs/fusefs/setattr.cc +++ b/tests/sys/fs/fusefs/setattr.cc @@ -322,7 +322,7 @@ TEST_F(Setattr, fchmod) fd = open(FULLPATH, O_RDONLY); ASSERT_LE(0, fd) << strerror(errno); ASSERT_EQ(0, fchmod(fd, newmode)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* Change the size of an open file, by its file descriptor */ @@ -376,7 +376,7 @@ TEST_F(Setattr, ftruncate) fd = open(FULLPATH, O_RDWR); ASSERT_LE(0, fd) << strerror(errno); ASSERT_EQ(0, ftruncate(fd, newsize)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* Change the size of the file */ diff --git a/tests/sys/fs/fusefs/unlink.cc b/tests/sys/fs/fusefs/unlink.cc index 2721ba78232..e4dd782240f 100644 --- a/tests/sys/fs/fusefs/unlink.cc +++ b/tests/sys/fs/fusefs/unlink.cc @@ -135,5 +135,5 @@ TEST_F(Unlink, open_but_deleted) fd = open(FULLPATH, O_RDWR); ASSERT_LE(0, fd) << strerror(errno); ASSERT_EQ(0, unlink(FULLPATH)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } diff --git a/tests/sys/fs/fusefs/utils.hh b/tests/sys/fs/fusefs/utils.hh index 2dbca6808b0..50b101e378c 100644 --- a/tests/sys/fs/fusefs/utils.hh +++ b/tests/sys/fs/fusefs/utils.hh @@ -30,6 +30,8 @@ struct _sem; typedef struct _sem sem_t; +struct _dirdesc; +typedef struct _dirdesc DIR; /* Nanoseconds to sleep, for tests that must */ #define NAP_NS (100'000'000) @@ -210,4 +212,23 @@ class FuseTest : public ::testing::Test { void fork(bool drop_privs, int *status, std::function parent_func, std::function child_func); + + /* + * Deliberately leak a file descriptor. + * + * Closing a file descriptor on fusefs would cause the server to + * receive FUSE_CLOSE and possibly FUSE_INACTIVE. Handling those + * operations would needlessly complicate most tests. So most tests + * deliberately leak the file descriptors instead. This method serves + * to document the leakage, and provide a single point of suppression + * for static analyzers. + */ + static void leak(int fd __unused) {} + + /* + * Deliberately leak a DIR* pointer + * + * See comments for FuseTest::leak + */ + static void leakdir(DIR* dirp __unused) {} }; diff --git a/tests/sys/fs/fusefs/write.cc b/tests/sys/fs/fusefs/write.cc index 8a4acf58caf..3641ae50fe2 100644 --- a/tests/sys/fs/fusefs/write.cc +++ b/tests/sys/fs/fusefs/write.cc @@ -231,7 +231,7 @@ TEST_F(AioWrite, DISABLED_aio_write) iocb.aio_sigevent.sigev_notify = SIGEV_NONE; ASSERT_EQ(0, aio_write(&iocb)) << strerror(errno); ASSERT_EQ(bufsize, aio_waitcomplete(&piocb, NULL)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* @@ -267,7 +267,7 @@ TEST_F(Write, append) EXPECT_LE(0, fd) << strerror(errno); ASSERT_EQ(BUFSIZE, write(fd, CONTENTS, BUFSIZE)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* If a file is cached, then appending to the end should not cause a read */ @@ -305,7 +305,7 @@ TEST_F(Write, append_to_cached) /* Write the new data. There should be no more read operations */ ASSERT_EQ(BUFSIZE, write(fd, CONTENTS, BUFSIZE)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } TEST_F(Write, append_direct_io) @@ -326,7 +326,7 @@ TEST_F(Write, append_direct_io) EXPECT_LE(0, fd) << strerror(errno); ASSERT_EQ(BUFSIZE, write(fd, CONTENTS, BUFSIZE)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* A direct write should evict any overlapping cached data */ @@ -364,7 +364,7 @@ TEST_F(Write, direct_io_evicts_cache) ASSERT_EQ(bufsize, read(fd, readbuf, bufsize)) << strerror(errno); ASSERT_STREQ(readbuf, CONTENTS1); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* @@ -394,7 +394,7 @@ TEST_F(Write, indirect_io_short_write) EXPECT_LE(0, fd) << strerror(errno); ASSERT_EQ(bufsize, write(fd, CONTENTS, bufsize)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* @@ -419,7 +419,7 @@ TEST_F(Write, direct_io_short_write) EXPECT_LE(0, fd) << strerror(errno); ASSERT_EQ(halfbufsize, write(fd, CONTENTS, bufsize)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* @@ -453,7 +453,7 @@ TEST_F(Write, direct_io_short_write_iov) iov[1].iov_base = (void*)CONTENTS1; iov[1].iov_len = strlen(CONTENTS1); ASSERT_EQ(size0, writev(fd, iov, 2)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* fusefs should respect RLIMIT_FSIZE */ @@ -483,7 +483,7 @@ TEST_F(Write, rlimit_fsize) ASSERT_EQ(-1, pwrite(fd, CONTENTS, bufsize, offset)); EXPECT_EQ(EFBIG, errno); EXPECT_EQ(1, s_sigxfsz); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* @@ -516,7 +516,7 @@ TEST_F(Write, eof_during_rmw) ASSERT_EQ(bufsize, pwrite(fd, CONTENTS, bufsize, offset)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* @@ -590,7 +590,7 @@ TEST_F(Write, pwrite) ASSERT_EQ(bufsize, pwrite(fd, CONTENTS, bufsize, offset)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* Writing a file should update its cached mtime and ctime */ @@ -639,7 +639,7 @@ TEST_F(Write, write) EXPECT_LE(0, fd) << strerror(errno); ASSERT_EQ(bufsize, write(fd, CONTENTS, bufsize)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* fuse(4) should not issue writes of greater size than the daemon requests */ @@ -670,7 +670,7 @@ TEST_F(Write, write_large) EXPECT_LE(0, fd) << strerror(errno); ASSERT_EQ(bufsize, write(fd, contents, bufsize)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); free(contents); } @@ -691,7 +691,7 @@ TEST_F(Write, write_nothing) EXPECT_LE(0, fd) << strerror(errno); ASSERT_EQ(bufsize, write(fd, CONTENTS, bufsize)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } TEST_F(Write_7_8, write) @@ -711,7 +711,7 @@ TEST_F(Write_7_8, write) EXPECT_LE(0, fd) << strerror(errno); ASSERT_EQ(bufsize, write(fd, CONTENTS, bufsize)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* In writeback mode, dirty data should be written on close */ @@ -855,7 +855,7 @@ TEST_F(WriteBack, rmw) ASSERT_EQ(bufsize, pwrite(fd, CONTENTS, bufsize, offset)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* @@ -885,7 +885,7 @@ TEST_F(WriteBack, cache) */ ASSERT_EQ(0, lseek(fd, 0, SEEK_SET)) << strerror(errno); ASSERT_EQ(bufsize, read(fd, readbuf, bufsize)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* @@ -917,7 +917,7 @@ TEST_F(WriteBack, o_direct) ASSERT_EQ(0, lseek(fd, 0, SEEK_SET)) << strerror(errno); ASSERT_EQ(0, fcntl(fd, F_SETFL, 0)) << strerror(errno); ASSERT_EQ(bufsize, read(fd, readbuf, bufsize)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* @@ -995,7 +995,7 @@ TEST_F(WriteBackAsync, eof) /* The file's size should still be what was established by pwrite */ ASSERT_EQ(0, fstat(fd, &sb)) << strerror(errno); EXPECT_EQ(offset + wbufsize, sb.st_size); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* @@ -1154,7 +1154,7 @@ TEST_F(Write, writethrough) */ ASSERT_EQ(0, lseek(fd, 0, SEEK_SET)) << strerror(errno); ASSERT_EQ(bufsize, read(fd, readbuf, bufsize)) << strerror(errno); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } /* Writes that extend a file should update the cached file size */ @@ -1179,5 +1179,5 @@ TEST_F(Write, update_file_size) /* Get cached attributes */ ASSERT_EQ(0, fstat(fd, &sb)) << strerror(errno); ASSERT_EQ(bufsize, sb.st_size); - /* Deliberately leak fd. close(2) will be tested in release.cc */ + leak(fd); } -- 2.45.0