Clean up how we skip tests when a syscall isn't in the kernel.
The close_range() test in particular has been confusing a lot of
partners. I think the sys_epoll_test.cpp idiom is the clearest of the
ones in use, so let's use that everywhere. (I haven't actually touched
the SysV IPC tests, because if we do touch them, _deleting_ them --
since all those syscalls are not allowed on Android -- is probably the
change to be made!)
I'm on the fence about factoring this idiom out into a macro. There
should never be too many of these, and we should probably be removing
them? Is anyone still running the current bionic tests on 4.3 kernels
without membarrier(2), and if they are --- why?!
For now though, I haven't removed any of our existing tests; I've just
moved them over to the sys_epoll_test.cpp style.
Test: treehugger
Change-Id: Ie69a0bb8f416c79957188e187610ff8a3c4d1e8f
diff --git a/tests/dlext_test.cpp b/tests/dlext_test.cpp
index ea28822..3c2dcf2 100644
--- a/tests/dlext_test.cpp
+++ b/tests/dlext_test.cpp
@@ -964,10 +964,7 @@
// create memfd
int memfd = memfd_create("foobar", MFD_CLOEXEC);
- if (memfd == -1 && errno == ENOSYS) {
- return;
- }
-
+ if (memfd == -1 && errno == ENOSYS) GTEST_SKIP() << "no memfd_create() in this kernel";
ASSERT_TRUE(memfd != -1) << strerror(errno);
// Check st.f_type is TMPFS_MAGIC for memfd
diff --git a/tests/fdtrack_test.cpp b/tests/fdtrack_test.cpp
index ff6d8c8..5988bc4 100644
--- a/tests/fdtrack_test.cpp
+++ b/tests/fdtrack_test.cpp
@@ -235,27 +235,22 @@
FDTRACK_TEST(pidfd_open, ({
int rc = pidfd_open(getpid(), 0);
- if (rc == -1) {
- ASSERT_ERRNO(ENOSYS);
- GTEST_SKIP() << "pidfd_open not available";
- }
+ if (rc == -1 && errno == ENOSYS) GTEST_SKIP() << "no pidfd_open() in this kernel";
+ ASSERT_NE(-1, rc) << strerror(errno);
rc;
}));
FDTRACK_TEST(pidfd_getfd, ({
android_fdtrack_set_enabled(false);
int pidfd_self = pidfd_open(getpid(), 0);
- if (pidfd_self == -1) {
- ASSERT_ERRNO(ENOSYS);
- GTEST_SKIP() << "pidfd_open not available";
- }
+ if (pidfd_self == -1 && errno == ENOSYS) GTEST_SKIP() << "no pidfd_open() in this kernel";
+ ASSERT_NE(-1, pidfd_self) << strerror(errno);
+
android_fdtrack_set_enabled(true);
int rc = pidfd_getfd(pidfd_self, STDIN_FILENO, 0);
- if (rc == -1) {
- ASSERT_ERRNO(ENOSYS);
- GTEST_SKIP() << "pidfd_getfd not available";
- }
+ if (rc == -1 && errno == ENOSYS) GTEST_SKIP() << "no pidfd_getfd() in this kernel";
+ ASSERT_NE(-1, rc) << strerror(errno);
android_fdtrack_set_enabled(false);
close(pidfd_self);
diff --git a/tests/membarrier_test.cpp b/tests/membarrier_test.cpp
index 891488b..0cb7df4 100644
--- a/tests/membarrier_test.cpp
+++ b/tests/membarrier_test.cpp
@@ -30,7 +30,7 @@
~ScopedErrnoCleaner() { errno = 0; }
};
-bool HasMembarrier(int membarrier_cmd) {
+static bool HasMembarrier(int membarrier_cmd) {
ScopedErrnoCleaner errno_cleaner;
int supported_cmds = syscall(__NR_membarrier, MEMBARRIER_CMD_QUERY, 0);
return (supported_cmds > 0) && ((supported_cmds & membarrier_cmd) != 0);
@@ -39,11 +39,8 @@
TEST(membarrier, query) {
ScopedErrnoCleaner errno_cleaner;
int supported = syscall(__NR_membarrier, MEMBARRIER_CMD_QUERY, 0);
- if (errno == 0) {
- ASSERT_TRUE(supported >= 0);
- } else {
- ASSERT_TRUE(errno == ENOSYS && supported == -1);
- }
+ if (supported == -1 && errno == ENOSYS) GTEST_SKIP() << "no membarrier() in this kernel";
+ ASSERT_GE(supported, 0);
}
TEST(membarrier, global_barrier) {
diff --git a/tests/pidfd_test.cpp b/tests/pidfd_test.cpp
index e2e2046..c01b93f 100644
--- a/tests/pidfd_test.cpp
+++ b/tests/pidfd_test.cpp
@@ -45,10 +45,8 @@
}
unique_fd pidfd(pidfd_open(child, 0));
- if (pidfd.get() == -1) {
- ASSERT_ERRNO(ENOSYS);
- GTEST_SKIP() << "pidfd_open not available";
- }
+ if (pidfd.get() == -1 && errno == ENOSYS) GTEST_SKIP() << "no pidfd_open() in this kernel";
+ ASSERT_NE(-1, pidfd.get()) << strerror(errno);
siginfo_t siginfo;
int rc = waitid(P_PIDFD, pidfd.get(), &siginfo, WEXITED);
@@ -66,16 +64,12 @@
unique_fd r, w;
ASSERT_TRUE(android::base::Pipe(&r, &w));
unique_fd self(pidfd_open(getpid(), 0));
- if (self.get() == -1) {
- ASSERT_ERRNO(ENOSYS);
- GTEST_SKIP() << "pidfd_open not available";
- }
+ if (self.get() == -1 && errno == ENOSYS) GTEST_SKIP() << "no pidfd_open() in this kernel";
+ ASSERT_NE(-1, self.get()) << strerror(errno);
unique_fd dup(pidfd_getfd(self.get(), r.get(), 0));
- if (dup.get() == -1) {
- ASSERT_ERRNO(ENOSYS);
- GTEST_SKIP() << "pidfd_getfd not available";
- }
+ if (dup.get() == -1 && errno == ENOSYS) GTEST_SKIP() << "no pidfd_getfd() in this kernel";
+ ASSERT_NE(-1, dup.get()) << strerror(errno);
ASSERT_NE(r.get(), dup.get());
ASSERT_EQ(3, write(w.get(), "foo", 3));
@@ -88,15 +82,12 @@
TEST_F(pidfd_DeathTest, pidfd_send_signal) {
#if defined(__BIONIC__)
unique_fd self(pidfd_open(getpid(), 0));
- if (self.get() == -1) {
- ASSERT_ERRNO(ENOSYS);
- GTEST_SKIP() << "pidfd_open not available";
- }
+ if (self.get() == -1 && errno == ENOSYS) GTEST_SKIP() << "no pidfd_open() in this kernel";
+ ASSERT_NE(-1, self.get()) << strerror(errno);
- if (pidfd_send_signal(self.get(), 0, nullptr, 0) == -1) {
- ASSERT_ERRNO(ENOSYS);
- GTEST_SKIP() << "pidfd_send_signal not available";
- }
+ int rc = pidfd_send_signal(self.get(), 0, nullptr, 0);
+ if (rc == -1 && errno == ENOSYS) GTEST_SKIP() << "no pidfd_send_signal() in this kernel";
+ ASSERT_EQ(0, rc) << strerror(errno);
ASSERT_EXIT(({
// gtest will fork a child off for ASSERT_EXIT: `self` refers to the parent.
diff --git a/tests/sys_epoll_test.cpp b/tests/sys_epoll_test.cpp
index abd928a..744f1c9 100644
--- a/tests/sys_epoll_test.cpp
+++ b/tests/sys_epoll_test.cpp
@@ -64,7 +64,7 @@
epoll_event events[1] = {};
timespec ts = {.tv_nsec = 500};
int rc = epoll_pwait2(epoll_fd, events, 1, &ts, nullptr);
- if (rc == -1 && errno == ENOSYS) GTEST_SKIP() << "no epoll_pwait2 in this kernel";
+ if (rc == -1 && errno == ENOSYS) GTEST_SKIP() << "no epoll_pwait2() in this kernel";
ASSERT_EQ(0, rc) << strerror(errno);
#else
GTEST_SKIP() << "epoll_pwait2 is only in glibc 2.35+";
@@ -94,7 +94,7 @@
sigemptyset(&ss2);
sigaddset(&ss2, SIGPIPE);
int rc = epoll_pwait2(epoll_fd, events, 1, &ts, &ss2);
- if (rc == -1 && errno == ENOSYS) GTEST_SKIP() << "no epoll_pwait2 in this kernel";
+ if (rc == -1 && errno == ENOSYS) GTEST_SKIP() << "no epoll_pwait2() in this kernel";
ASSERT_EQ(0, rc) << strerror(errno);
#else
GTEST_SKIP() << "epoll_pwait2 is only in glibc 2.35+";
@@ -112,7 +112,7 @@
sigemptyset64(&ss2);
sigaddset64(&ss2, SIGPIPE);
int rc = epoll_pwait2_64(epoll_fd, events, 1, &ts, &ss2);
- if (rc == -1 && errno == ENOSYS) GTEST_SKIP() << "no epoll_pwait2 in this kernel";
+ if (rc == -1 && errno == ENOSYS) GTEST_SKIP() << "no epoll_pwait2() in this kernel";
ASSERT_EQ(0, rc) << strerror(errno);
#else
GTEST_SKIP() << "epoll_pwait2_64 is bionic-only";
diff --git a/tests/sys_mman_test.cpp b/tests/sys_mman_test.cpp
index 9421565..e785ff4 100644
--- a/tests/sys_mman_test.cpp
+++ b/tests/sys_mman_test.cpp
@@ -279,10 +279,9 @@
// Is the MFD_CLOEXEC flag obeyed?
errno = 0;
int fd = memfd_create("doesn't matter", 0);
- if (fd == -1) {
- ASSERT_ERRNO(ENOSYS);
- GTEST_SKIP() << "no memfd_create available";
- }
+ if (fd == -1 && errno == ENOSYS) GTEST_SKIP() << "no memfd_create() in this kernel";
+ ASSERT_NE(-1, fd) << strerror(errno);
+
int f = fcntl(fd, F_GETFD);
ASSERT_NE(-1, f);
ASSERT_FALSE(f & FD_CLOEXEC);
diff --git a/tests/sys_stat_test.cpp b/tests/sys_stat_test.cpp
index b00f6e2..126f469 100644
--- a/tests/sys_stat_test.cpp
+++ b/tests/sys_stat_test.cpp
@@ -109,9 +109,7 @@
#if defined(HAVE_STATX)
struct statx sx;
int rc = statx(AT_FDCWD, "/proc/version", AT_STATX_SYNC_AS_STAT, STATX_ALL, &sx);
- if (rc == -1 && errno == ENOSYS) {
- GTEST_SKIP() << "statx returned ENOSYS";
- }
+ if (rc == -1 && errno == ENOSYS) GTEST_SKIP() << "no statx() in this kernel";
ASSERT_EQ(0, rc);
struct stat64 sb;
ASSERT_EQ(0, stat64("/proc/version", &sb));
diff --git a/tests/unistd_test.cpp b/tests/unistd_test.cpp
index 74bd2f4..6a94507 100644
--- a/tests/unistd_test.cpp
+++ b/tests/unistd_test.cpp
@@ -1689,16 +1689,13 @@
int fd = open("/proc/version", O_RDONLY);
ASSERT_GE(fd, 0);
- // Try to close the file descriptor (this requires a 5.9+ kernel)
- if (close_range(fd, fd, 0) == 0) {
- // we can't close it *again*
- ASSERT_EQ(close(fd), -1);
- ASSERT_ERRNO(EBADF);
- } else {
- ASSERT_ERRNO(ENOSYS);
- // since close_range() failed, we can close it normally
- ASSERT_EQ(close(fd), 0);
- }
+ int rc = close_range(fd, fd, 0);
+ if (rc == -1 && errno == ENOSYS) GTEST_SKIP() << "no close_range() in this kernel";
+ ASSERT_EQ(0, rc) << strerror(errno);
+
+ // Check the fd is actually closed.
+ ASSERT_EQ(close(fd), -1);
+ ASSERT_ERRNO(EBADF);
#endif // __GLIBC__
}