Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libc][NFC] Use specific EXPECT/ASSERT macros to test errno #79573

Merged
merged 3 commits into from
Jan 26, 2024

Conversation

gchatelet
Copy link
Contributor

No description provided.

@gchatelet gchatelet requested a review from lntue January 26, 2024 10:35
@llvmbot llvmbot added the libc label Jan 26, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 26, 2024

@llvm/pr-subscribers-libc

Author: Guillaume Chatelet (gchatelet)

Changes

Patch is 80.58 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/79573.diff

57 Files Affected:

  • (modified) libc/test/IntegrationTest/test.h (+16)
  • (modified) libc/test/UnitTest/LibcTest.h (+9)
  • (modified) libc/test/integration/src/pthread/pthread_create_test.cpp (+26-26)
  • (modified) libc/test/integration/src/pthread/pthread_join_test.cpp (+2-2)
  • (modified) libc/test/integration/src/unistd/getcwd_test.cpp (+1-1)
  • (modified) libc/test/src/dirent/dirent_test.cpp (+3-3)
  • (modified) libc/test/src/fcntl/creat_test.cpp (+2-2)
  • (modified) libc/test/src/fcntl/openat_test.cpp (+2-2)
  • (modified) libc/test/src/sched/get_priority_test.cpp (+14-14)
  • (modified) libc/test/src/sched/param_and_scheduler_test.cpp (+17-17)
  • (modified) libc/test/src/sched/sched_rr_get_interval_test.cpp (+7-7)
  • (modified) libc/test/src/sched/yield_test.cpp (+1-1)
  • (modified) libc/test/src/stdio/fgets_test.cpp (+3-3)
  • (modified) libc/test/src/stdio/fileop_test.cpp (+3-3)
  • (modified) libc/test/src/stdio/fopencookie_test.cpp (+3-3)
  • (modified) libc/test/src/stdio/remove_test.cpp (+1-1)
  • (modified) libc/test/src/stdio/setvbuf_test.cpp (+1-1)
  • (modified) libc/test/src/stdio/unlocked_fileop_test.cpp (+2-2)
  • (modified) libc/test/src/stdlib/StrtolTest.h (+53-53)
  • (modified) libc/test/src/stdlib/strtof_test.cpp (+1-1)
  • (modified) libc/test/src/stdlib/strtold_test.cpp (+1-1)
  • (modified) libc/test/src/string/strdup_test.cpp (+3-3)
  • (modified) libc/test/src/sys/prctl/linux/prctl_test.cpp (+1-1)
  • (modified) libc/test/src/sys/resource/getrlimit_setrlimit_test.cpp (+6-6)
  • (modified) libc/test/src/sys/sendfile/sendfile_test.cpp (+4-4)
  • (modified) libc/test/src/sys/socket/linux/bind_test.cpp (+2-2)
  • (modified) libc/test/src/sys/socket/linux/socket_test.cpp (+1-1)
  • (modified) libc/test/src/sys/stat/chmod_test.cpp (+4-4)
  • (modified) libc/test/src/sys/stat/fchmod_test.cpp (+5-5)
  • (modified) libc/test/src/sys/stat/fchmodat_test.cpp (+4-4)
  • (modified) libc/test/src/sys/stat/fstat_test.cpp (+1-1)
  • (modified) libc/test/src/sys/stat/lstat_test.cpp (+1-1)
  • (modified) libc/test/src/sys/stat/stat_test.cpp (+1-1)
  • (modified) libc/test/src/termios/termios_test.cpp (+2-2)
  • (modified) libc/test/src/time/gmtime_test.cpp (+2-2)
  • (modified) libc/test/src/time/nanosleep_test.cpp (+1-1)
  • (modified) libc/test/src/unistd/access_test.cpp (+8-8)
  • (modified) libc/test/src/unistd/chdir_test.cpp (+2-2)
  • (modified) libc/test/src/unistd/dup2_test.cpp (+4-4)
  • (modified) libc/test/src/unistd/dup3_test.cpp (+4-4)
  • (modified) libc/test/src/unistd/dup_test.cpp (+4-4)
  • (modified) libc/test/src/unistd/fchdir_test.cpp (+4-4)
  • (modified) libc/test/src/unistd/ftruncate_test.cpp (+4-4)
  • (modified) libc/test/src/unistd/isatty_test.cpp (+2-2)
  • (modified) libc/test/src/unistd/link_test.cpp (+2-2)
  • (modified) libc/test/src/unistd/linkat_test.cpp (+2-2)
  • (modified) libc/test/src/unistd/lseek_test.cpp (+2-2)
  • (modified) libc/test/src/unistd/pread_pwrite_test.cpp (+3-3)
  • (modified) libc/test/src/unistd/read_write_test.cpp (+2-2)
  • (modified) libc/test/src/unistd/readlink_test.cpp (+1-1)
  • (modified) libc/test/src/unistd/readlinkat_test.cpp (+1-1)
  • (modified) libc/test/src/unistd/symlink_test.cpp (+2-2)
  • (modified) libc/test/src/unistd/symlinkat_test.cpp (+2-2)
  • (modified) libc/test/src/unistd/syscall_test.cpp (+16-16)
  • (modified) libc/test/src/unistd/truncate_test.cpp (+3-3)
  • (modified) libc/test/src/unistd/unlink_test.cpp (+1-1)
  • (modified) libc/test/src/unistd/unlinkat_test.cpp (+3-3)
diff --git a/libc/test/IntegrationTest/test.h b/libc/test/IntegrationTest/test.h
index 1e5ad1cfef0d7b5..f1c2e8f9ec72e3c 100644
--- a/libc/test/IntegrationTest/test.h
+++ b/libc/test/IntegrationTest/test.h
@@ -45,10 +45,17 @@
       LIBC_NAMESPACE::quick_exit(127);                                         \
   }
 
+////////////////////////////////////////////////////////////////////////////////
+// Boolean checks are handled as comparison to the true / false values.
+
 #define EXPECT_TRUE(val) __CHECK_TRUE(__FILE__, __LINE__, val, false)
 #define ASSERT_TRUE(val) __CHECK_TRUE(__FILE__, __LINE__, val, true)
 #define EXPECT_FALSE(val) __CHECK_FALSE(__FILE__, __LINE__, val, false)
 #define ASSERT_FALSE(val) __CHECK_FALSE(__FILE__, __LINE__, val, true)
+
+////////////////////////////////////////////////////////////////////////////////
+// Binary equality / inequality.
+
 #define EXPECT_EQ(val1, val2)                                                  \
   __CHECK_EQ(__FILE__, __LINE__, (val1), (val2), false)
 #define ASSERT_EQ(val1, val2)                                                  \
@@ -58,6 +65,15 @@
 #define ASSERT_NE(val1, val2)                                                  \
   __CHECK_NE(__FILE__, __LINE__, (val1), (val2), true)
 
+////////////////////////////////////////////////////////////////////////////////
+// Errno checks.
+
+#define EXPECT_ERRNO_EQ(VAL) EXPECT_EQ(VAL, static_cast<int>(libc_errno))
+#define ASSERT_ERRNO_EQ(VAL) ASSERT_EQ(VAL, static_cast<int>(libc_errno))
+
+#define EXPECT_ERRNO() EXPECT_NE(static_cast<int>(libc_errno), 0)
+#define ASSERT_ERRNO() ASSERT_NE(static_cast<int>(libc_errno), 0)
+
 // Integration tests are compiled with -ffreestanding which stops treating
 // the main function as a non-overloadable special function. Hence, we use a
 // convenience macro which declares it 'extern "C"'.
diff --git a/libc/test/UnitTest/LibcTest.h b/libc/test/UnitTest/LibcTest.h
index 68fd5f28f3e0bd7..cfea23a679755ea 100644
--- a/libc/test/UnitTest/LibcTest.h
+++ b/libc/test/UnitTest/LibcTest.h
@@ -443,6 +443,15 @@ CString libc_make_test_file_path_func(const char *file_name);
 #define EXPECT_STRNE(LHS, RHS) LIBC_TEST_STR_(testStrNe, LHS, RHS, )
 #define ASSERT_STRNE(LHS, RHS) LIBC_TEST_STR_(testStrNe, LHS, RHS, return)
 
+////////////////////////////////////////////////////////////////////////////////
+// Errno checks.
+
+#define EXPECT_ERRNO_EQ(VAL) EXPECT_EQ(VAL, static_cast<int>(libc_errno))
+#define ASSERT_ERRNO_EQ(VAL) ASSERT_EQ(VAL, static_cast<int>(libc_errno))
+
+#define EXPECT_ERRNO() EXPECT_NE(static_cast<int>(libc_errno), 0)
+#define ASSERT_ERRNO() ASSERT_NE(static_cast<int>(libc_errno), 0)
+
 ////////////////////////////////////////////////////////////////////////////////
 // Subprocess checks.
 
diff --git a/libc/test/integration/src/pthread/pthread_create_test.cpp b/libc/test/integration/src/pthread/pthread_create_test.cpp
index 6a9b44cc1422cc5..bce380fbec2fc81 100644
--- a/libc/test/integration/src/pthread/pthread_create_test.cpp
+++ b/libc/test/integration/src/pthread/pthread_create_test.cpp
@@ -47,7 +47,7 @@ static void *successThread(void *Arg) {
   pthread_t th = LIBC_NAMESPACE::pthread_self();
   auto *thread = reinterpret_cast<LIBC_NAMESPACE::Thread *>(&th);
 
-  ASSERT_EQ(libc_errno, 0);
+  ASSERT_ERRNO_EQ(0);
   ASSERT_TRUE(thread);
   ASSERT_TRUE(thread->attrib);
 
@@ -62,22 +62,22 @@ static void *successThread(void *Arg) {
   ASSERT_EQ(LIBC_NAMESPACE::pthread_attr_getstack(expec_attrs, &expec_stack,
                                                   &expec_stacksize),
             0);
-  ASSERT_EQ(libc_errno, 0);
+  ASSERT_ERRNO_EQ(0);
 
   ASSERT_EQ(
       LIBC_NAMESPACE::pthread_attr_getstacksize(expec_attrs, &expec_stacksize2),
       0);
-  ASSERT_EQ(libc_errno, 0);
+  ASSERT_ERRNO_EQ(0);
 
   ASSERT_EQ(
       LIBC_NAMESPACE::pthread_attr_getguardsize(expec_attrs, &expec_guardsize),
       0);
-  ASSERT_EQ(libc_errno, 0);
+  ASSERT_ERRNO_EQ(0);
 
   ASSERT_EQ(
       LIBC_NAMESPACE::pthread_attr_getdetachstate(expec_attrs, &expec_detached),
       0);
-  ASSERT_EQ(libc_errno, 0);
+  ASSERT_ERRNO_EQ(0);
 
   ASSERT_EQ(expec_stacksize, expec_stacksize2);
 
@@ -125,7 +125,7 @@ static void *successThread(void *Arg) {
   // permissions. Maybe we can read from /proc/{self}/map?
 
   ASSERT_EQ(LIBC_NAMESPACE::pthread_attr_destroy(expec_attrs), 0);
-  ASSERT_EQ(libc_errno, 0);
+  ASSERT_ERRNO_EQ(0);
 
   // Arg is malloced, so free.
   delete th_arg;
@@ -140,13 +140,13 @@ static void run_success_config(int detachstate, size_t guardsize,
   pthread_attr_t *attr = &(th_arg->attrs);
 
   ASSERT_EQ(LIBC_NAMESPACE::pthread_attr_init(attr), 0);
-  ASSERT_EQ(libc_errno, 0);
+  ASSERT_ERRNO_EQ(0);
 
   ASSERT_EQ(LIBC_NAMESPACE::pthread_attr_setdetachstate(attr, detachstate), 0);
-  ASSERT_EQ(libc_errno, 0);
+  ASSERT_ERRNO_EQ(0);
 
   ASSERT_EQ(LIBC_NAMESPACE::pthread_attr_setguardsize(attr, guardsize), 0);
-  ASSERT_EQ(libc_errno, 0);
+  ASSERT_ERRNO_EQ(0);
 
   void *Stack = nullptr;
   if (customstack) {
@@ -154,20 +154,20 @@ static void run_success_config(int detachstate, size_t guardsize,
                                  MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
     ASSERT_NE(Stack, MAP_FAILED);
     ASSERT_NE(Stack, static_cast<void *>(nullptr));
-    ASSERT_EQ(libc_errno, 0);
+    ASSERT_ERRNO_EQ(0);
 
     ASSERT_EQ(LIBC_NAMESPACE::pthread_attr_setstack(attr, Stack, stacksize), 0);
-    ASSERT_EQ(libc_errno, 0);
+    ASSERT_ERRNO_EQ(0);
   } else {
     ASSERT_EQ(LIBC_NAMESPACE::pthread_attr_setstacksize(attr, stacksize), 0);
-    ASSERT_EQ(libc_errno, 0);
+    ASSERT_ERRNO_EQ(0);
   }
 
   void *expec_ret = nullptr;
   if (detachstate == PTHREAD_CREATE_JOINABLE) {
     ASSERT_EQ(LIBC_NAMESPACE::getrandom(&expec_ret, sizeof(expec_ret), 0),
               static_cast<ssize_t>(sizeof(expec_ret)));
-    ASSERT_EQ(libc_errno, 0);
+    ASSERT_ERRNO_EQ(0);
   }
 
   th_arg->ret = expec_ret;
@@ -178,17 +178,17 @@ static void run_success_config(int detachstate, size_t guardsize,
   ASSERT_EQ(LIBC_NAMESPACE::pthread_create(&tid, attr, successThread,
                                            reinterpret_cast<void *>(th_arg)),
             0);
-  ASSERT_EQ(libc_errno, 0);
+  ASSERT_ERRNO_EQ(0);
 
   if (detachstate == PTHREAD_CREATE_JOINABLE) {
     void *th_ret;
     ASSERT_EQ(LIBC_NAMESPACE::pthread_join(tid, &th_ret), 0);
-    ASSERT_EQ(libc_errno, 0);
+    ASSERT_ERRNO_EQ(0);
     ASSERT_EQ(th_ret, expec_ret);
 
     if (customstack) {
       ASSERT_EQ(LIBC_NAMESPACE::munmap(Stack, stacksize), 0);
-      ASSERT_EQ(libc_errno, 0);
+      ASSERT_ERRNO_EQ(0);
     }
   } else {
     ASSERT_FALSE(customstack);
@@ -260,23 +260,23 @@ static void create_and_check_failure_thread(pthread_attr_t *attr) {
   // was just really larger we failed mmap.
   ASSERT_TRUE(result == EINVAL || result == EAGAIN);
   // pthread_create should NOT set errno on error
-  ASSERT_EQ(libc_errno, 0);
+  ASSERT_ERRNO_EQ(0);
 
   ASSERT_EQ(LIBC_NAMESPACE::pthread_attr_destroy(attr), 0);
-  ASSERT_EQ(libc_errno, 0);
+  ASSERT_ERRNO_EQ(0);
 }
 
 static void run_failure_config(size_t guardsize, size_t stacksize) {
   pthread_attr_t attr;
   guardsize &= -EXEC_PAGESIZE;
   ASSERT_EQ(LIBC_NAMESPACE::pthread_attr_init(&attr), 0);
-  ASSERT_EQ(libc_errno, 0);
+  ASSERT_ERRNO_EQ(0);
 
   ASSERT_EQ(LIBC_NAMESPACE::pthread_attr_setguardsize(&attr, guardsize), 0);
-  ASSERT_EQ(libc_errno, 0);
+  ASSERT_ERRNO_EQ(0);
 
   ASSERT_EQ(LIBC_NAMESPACE::pthread_attr_setstacksize(&attr, stacksize), 0);
-  ASSERT_EQ(libc_errno, 0);
+  ASSERT_ERRNO_EQ(0);
 
   create_and_check_failure_thread(&attr);
 }
@@ -301,32 +301,32 @@ static void run_failure_tests() {
 
   // Stacksize too small.
   ASSERT_EQ(LIBC_NAMESPACE::pthread_attr_init(&attr), 0);
-  ASSERT_EQ(libc_errno, 0);
+  ASSERT_ERRNO_EQ(0);
   attr.__stacksize = PTHREAD_STACK_MIN - 16;
   create_and_check_failure_thread(&attr);
 
   // Stack misaligned.
   ASSERT_EQ(LIBC_NAMESPACE::pthread_attr_init(&attr), 0);
-  ASSERT_EQ(libc_errno, 0);
+  ASSERT_ERRNO_EQ(0);
   attr.__stack = reinterpret_cast<void *>(1);
   create_and_check_failure_thread(&attr);
 
   // Stack + stacksize misaligned.
   ASSERT_EQ(LIBC_NAMESPACE::pthread_attr_init(&attr), 0);
-  ASSERT_EQ(libc_errno, 0);
+  ASSERT_ERRNO_EQ(0);
   attr.__stacksize = PTHREAD_STACK_MIN + 1;
   attr.__stack = reinterpret_cast<void *>(16);
   create_and_check_failure_thread(&attr);
 
   // Guardsize misaligned.
   ASSERT_EQ(LIBC_NAMESPACE::pthread_attr_init(&attr), 0);
-  ASSERT_EQ(libc_errno, 0);
+  ASSERT_ERRNO_EQ(0);
   attr.__guardsize = EXEC_PAGESIZE / 2;
   create_and_check_failure_thread(&attr);
 
   // Detachstate is unknown.
   ASSERT_EQ(LIBC_NAMESPACE::pthread_attr_init(&attr), 0);
-  ASSERT_EQ(libc_errno, 0);
+  ASSERT_ERRNO_EQ(0);
   attr.__detachstate = -1;
   create_and_check_failure_thread(&attr);
 }
diff --git a/libc/test/integration/src/pthread/pthread_join_test.cpp b/libc/test/integration/src/pthread/pthread_join_test.cpp
index f4126357a0579dc..b441917a4cac29f 100644
--- a/libc/test/integration/src/pthread/pthread_join_test.cpp
+++ b/libc/test/integration/src/pthread/pthread_join_test.cpp
@@ -19,9 +19,9 @@ static void nullJoinTest() {
   pthread_t Tid;
   ASSERT_EQ(LIBC_NAMESPACE::pthread_create(&Tid, nullptr, simpleFunc, nullptr),
             0);
-  ASSERT_EQ(libc_errno, 0);
+  ASSERT_ERRNO_EQ(0);
   ASSERT_EQ(LIBC_NAMESPACE::pthread_join(Tid, nullptr), 0);
-  ASSERT_EQ(libc_errno, 0);
+  ASSERT_ERRNO_EQ(0);
 }
 
 TEST_MAIN() {
diff --git a/libc/test/integration/src/unistd/getcwd_test.cpp b/libc/test/integration/src/unistd/getcwd_test.cpp
index 3d8f5821a69efcb..87687d09b9e7aff 100644
--- a/libc/test/integration/src/unistd/getcwd_test.cpp
+++ b/libc/test/integration/src/unistd/getcwd_test.cpp
@@ -30,7 +30,7 @@ TEST_MAIN(int argc, char **argv, char **envp) {
   // Bad size
   cwd = LIBC_NAMESPACE::getcwd(buffer, 0);
   ASSERT_TRUE(cwd == nullptr);
-  ASSERT_EQ(libc_errno, EINVAL);
+  ASSERT_ERRNO_EQ(EINVAL);
   libc_errno = 0;
 
   // Insufficient size
diff --git a/libc/test/src/dirent/dirent_test.cpp b/libc/test/src/dirent/dirent_test.cpp
index ff1a30a2639fe25..63778ddc01483d9 100644
--- a/libc/test/src/dirent/dirent_test.cpp
+++ b/libc/test/src/dirent/dirent_test.cpp
@@ -44,7 +44,7 @@ TEST(LlvmLibcDirentTest, SimpleOpenAndRead) {
   }
 
   // Verify that we don't break out of the above loop in error.
-  ASSERT_EQ(libc_errno, 0);
+  ASSERT_ERRNO_EQ(0);
 
   ASSERT_TRUE(file1 != nullptr);
   ASSERT_TRUE(file2 != nullptr);
@@ -58,7 +58,7 @@ TEST(LlvmLibcDirentTest, OpenNonExistentDir) {
   libc_errno = 0;
   ::DIR *dir = LIBC_NAMESPACE::opendir("___xyz123__.non_existent__");
   ASSERT_TRUE(dir == nullptr);
-  ASSERT_EQ(libc_errno, ENOENT);
+  ASSERT_ERRNO_EQ(ENOENT);
   libc_errno = 0;
 }
 
@@ -66,6 +66,6 @@ TEST(LlvmLibcDirentTest, OpenFile) {
   libc_errno = 0;
   ::DIR *dir = LIBC_NAMESPACE::opendir("testdata/file1.txt");
   ASSERT_TRUE(dir == nullptr);
-  ASSERT_EQ(libc_errno, ENOTDIR);
+  ASSERT_ERRNO_EQ(ENOTDIR);
   libc_errno = 0;
 }
diff --git a/libc/test/src/fcntl/creat_test.cpp b/libc/test/src/fcntl/creat_test.cpp
index ef30d8862c45f26..9f5a1394dbfcd14 100644
--- a/libc/test/src/fcntl/creat_test.cpp
+++ b/libc/test/src/fcntl/creat_test.cpp
@@ -19,12 +19,12 @@ TEST(LlvmLibcCreatTest, CreatAndOpen) {
   using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds;
   constexpr const char *TEST_FILE = "testdata/creat.test";
   int fd = LIBC_NAMESPACE::creat(TEST_FILE, S_IRWXU);
-  ASSERT_EQ(libc_errno, 0);
+  ASSERT_ERRNO_EQ(0);
   ASSERT_GT(fd, 0);
   ASSERT_THAT(LIBC_NAMESPACE::close(fd), Succeeds(0));
 
   fd = LIBC_NAMESPACE::open(TEST_FILE, O_RDONLY);
-  ASSERT_EQ(libc_errno, 0);
+  ASSERT_ERRNO_EQ(0);
   ASSERT_GT(fd, 0);
   ASSERT_THAT(LIBC_NAMESPACE::close(fd), Succeeds(0));
 
diff --git a/libc/test/src/fcntl/openat_test.cpp b/libc/test/src/fcntl/openat_test.cpp
index b95f3f212720116..a6302973b8e2f35 100644
--- a/libc/test/src/fcntl/openat_test.cpp
+++ b/libc/test/src/fcntl/openat_test.cpp
@@ -21,13 +21,13 @@ TEST(LlvmLibcUniStd, OpenAndReadTest) {
   constexpr const char *TEST_DIR = "testdata";
   constexpr const char *TEST_FILE = "openat.test";
   int dir_fd = LIBC_NAMESPACE::open(TEST_DIR, O_DIRECTORY);
-  ASSERT_EQ(libc_errno, 0);
+  ASSERT_ERRNO_EQ(0);
   ASSERT_GT(dir_fd, 0);
   constexpr const char TEST_MSG[] = "openat test";
   constexpr int TEST_MSG_SIZE = sizeof(TEST_MSG) - 1;
 
   int read_fd = LIBC_NAMESPACE::openat(dir_fd, TEST_FILE, O_RDONLY);
-  ASSERT_EQ(libc_errno, 0);
+  ASSERT_ERRNO_EQ(0);
   ASSERT_GT(read_fd, 0);
   char read_buf[TEST_MSG_SIZE];
   ASSERT_THAT(LIBC_NAMESPACE::read(read_fd, read_buf, TEST_MSG_SIZE),
diff --git a/libc/test/src/sched/get_priority_test.cpp b/libc/test/src/sched/get_priority_test.cpp
index 3a79a67802cf3e9..6d62391ee2cb1b5 100644
--- a/libc/test/src/sched/get_priority_test.cpp
+++ b/libc/test/src/sched/get_priority_test.cpp
@@ -20,40 +20,40 @@ TEST(LlvmLibcSchedGetPriorityTest, HandleBadPolicyTest) {
     int policy = -1;
     int max_priority = LIBC_NAMESPACE::sched_get_priority_max(policy);
     ASSERT_EQ(max_priority, -1);
-    ASSERT_EQ(libc_errno, EINVAL);
+    ASSERT_ERRNO_EQ(EINVAL);
     int min_priority = LIBC_NAMESPACE::sched_get_priority_min(policy);
     ASSERT_EQ(min_priority, -1);
-    ASSERT_EQ(libc_errno, EINVAL);
+    ASSERT_ERRNO_EQ(EINVAL);
   }
 
   {
     int policy = 30;
     int max_priority = LIBC_NAMESPACE::sched_get_priority_max(policy);
     ASSERT_EQ(max_priority, -1);
-    ASSERT_EQ(libc_errno, EINVAL);
+    ASSERT_ERRNO_EQ(EINVAL);
     int min_priority = LIBC_NAMESPACE::sched_get_priority_min(policy);
     ASSERT_EQ(min_priority, -1);
-    ASSERT_EQ(libc_errno, EINVAL);
+    ASSERT_ERRNO_EQ(EINVAL);
   }
 
   {
     int policy = 80;
     int max_priority = LIBC_NAMESPACE::sched_get_priority_max(policy);
     ASSERT_EQ(max_priority, -1);
-    ASSERT_EQ(libc_errno, EINVAL);
+    ASSERT_ERRNO_EQ(EINVAL);
     int min_priority = LIBC_NAMESPACE::sched_get_priority_min(policy);
     ASSERT_EQ(min_priority, -1);
-    ASSERT_EQ(libc_errno, EINVAL);
+    ASSERT_ERRNO_EQ(EINVAL);
   }
 
   {
     int policy = 110;
     int max_priority = LIBC_NAMESPACE::sched_get_priority_max(policy);
     ASSERT_EQ(max_priority, -1);
-    ASSERT_EQ(libc_errno, EINVAL);
+    ASSERT_ERRNO_EQ(EINVAL);
     int min_priority = LIBC_NAMESPACE::sched_get_priority_min(policy);
     ASSERT_EQ(min_priority, -1);
-    ASSERT_EQ(libc_errno, EINVAL);
+    ASSERT_ERRNO_EQ(EINVAL);
   }
 }
 
@@ -68,10 +68,10 @@ TEST(LlvmLibcSchedGetPriorityTest, SmokeTest) {
     int policy = SCHED_OTHER;
     int max_priority = LIBC_NAMESPACE::sched_get_priority_max(policy);
     ASSERT_GE(max_priority, 0);
-    ASSERT_EQ(libc_errno, 0);
+    ASSERT_ERRNO_EQ(0);
     int min_priority = LIBC_NAMESPACE::sched_get_priority_min(policy);
     ASSERT_GE(min_priority, 0);
-    ASSERT_EQ(libc_errno, 0);
+    ASSERT_ERRNO_EQ(0);
 
     ASSERT_LE(max_priority, 99);
     ASSERT_GE(min_priority, 0);
@@ -82,10 +82,10 @@ TEST(LlvmLibcSchedGetPriorityTest, SmokeTest) {
     int policy = SCHED_FIFO;
     int max_priority = LIBC_NAMESPACE::sched_get_priority_max(policy);
     ASSERT_GE(max_priority, 0);
-    ASSERT_EQ(libc_errno, 0);
+    ASSERT_ERRNO_EQ(0);
     int min_priority = LIBC_NAMESPACE::sched_get_priority_min(policy);
     ASSERT_GE(min_priority, 0);
-    ASSERT_EQ(libc_errno, 0);
+    ASSERT_ERRNO_EQ(0);
 
     ASSERT_LE(max_priority, 99);
     ASSERT_GE(min_priority, 0);
@@ -96,10 +96,10 @@ TEST(LlvmLibcSchedGetPriorityTest, SmokeTest) {
     int policy = SCHED_RR;
     int max_priority = LIBC_NAMESPACE::sched_get_priority_max(policy);
     ASSERT_GE(max_priority, 0);
-    ASSERT_EQ(libc_errno, 0);
+    ASSERT_ERRNO_EQ(0);
     int min_priority = LIBC_NAMESPACE::sched_get_priority_min(policy);
     ASSERT_GE(min_priority, 0);
-    ASSERT_EQ(libc_errno, 0);
+    ASSERT_ERRNO_EQ(0);
 
     ASSERT_LE(max_priority, 99);
     ASSERT_GE(min_priority, 0);
diff --git a/libc/test/src/sched/param_and_scheduler_test.cpp b/libc/test/src/sched/param_and_scheduler_test.cpp
index 8c6485f07897125..196db53eb33be3b 100644
--- a/libc/test/src/sched/param_and_scheduler_test.cpp
+++ b/libc/test/src/sched/param_and_scheduler_test.cpp
@@ -41,35 +41,35 @@ class SchedTest : public LIBC_NAMESPACE::testing::Test {
 
     int init_policy = LIBC_NAMESPACE::sched_getscheduler(0);
     ASSERT_GE(init_policy, 0);
-    ASSERT_EQ(libc_errno, 0);
+    ASSERT_ERRNO_EQ(0);
 
     int max_priority = LIBC_NAMESPACE::sched_get_priority_max(policy);
     ASSERT_GE(max_priority, 0);
-    ASSERT_EQ(libc_errno, 0);
+    ASSERT_ERRNO_EQ(0);
     int min_priority = LIBC_NAMESPACE::sched_get_priority_min(policy);
     ASSERT_GE(min_priority, 0);
-    ASSERT_EQ(libc_errno, 0);
+    ASSERT_ERRNO_EQ(0);
 
     struct sched_param param = {min_priority};
 
     // Negative pid
     ASSERT_EQ(LIBC_NAMESPACE::sched_setscheduler(-1, policy, &param), -1);
-    ASSERT_EQ(libc_errno, EINVAL);
+    ASSERT_ERRNO_EQ(EINVAL);
     libc_errno = 0;
 
     ASSERT_EQ(LIBC_NAMESPACE::sched_getscheduler(-1), -1);
-    ASSERT_EQ(libc_errno, EINVAL);
+    ASSERT_ERRNO_EQ(EINVAL);
     libc_errno = 0;
 
     // Invalid Policy
     ASSERT_EQ(LIBC_NAMESPACE::sched_setscheduler(0, policy | 128, &param), -1);
-    ASSERT_EQ(libc_errno, EINVAL);
+    ASSERT_ERRNO_EQ(EINVAL);
     libc_errno = 0;
 
     // Out of bounds priority
     param.sched_priority = min_priority - 1;
     ASSERT_EQ(LIBC_NAMESPACE::sched_setscheduler(0, policy, &param), -1);
-    ASSERT_EQ(libc_errno, EINVAL);
+    ASSERT_ERRNO_EQ(EINVAL);
     libc_errno = 0;
 
     param.sched_priority = max_priority + 1;
@@ -90,33 +90,33 @@ class SchedTest : public LIBC_NAMESPACE::testing::Test {
 
     ASSERT_EQ(LIBC_NAMESPACE::sched_getscheduler(0),
               can_set ? policy : init_policy);
-    ASSERT_EQ(libc_errno, 0);
+    ASSERT_ERRNO_EQ(0);
 
     // Out of bounds priority
     param.sched_priority = -1;
     ASSERT_EQ(LIBC_NAMESPACE::sched_setparam(0, &param), -1);
-    ASSERT_EQ(libc_errno, EINVAL);
+    ASSERT_ERRNO_EQ(EINVAL);
     libc_errno = 0;
 
     param.sched_priority = max_priority + 1;
     ASSERT_EQ(LIBC_NAMESPACE::sched_setparam(0, &param), -1);
-    ASSERT_EQ(libc_errno, EINVAL);
+    ASSERT_ERRNO_EQ(EINVAL);
     libc_errno = 0;
 
     for (int priority = min_priority; priority <= max_priority; ++priority) {
       ASSERT_EQ(LIBC_NAMESPACE::sched_getparam(0, &param), 0);
-      ASSERT_EQ(libc_errno, 0);
+      ASSERT_ERRNO_EQ(0);
       int init_priority = param.sched_priority;
 
       param.sched_priority = priority;
 
       // Negative pid
       ASSERT_EQ(LIBC_NAMESPACE::sched_setparam(-1, &param), -1);
-      ASSERT_EQ(libc_errno, EINVAL);
+      ASSERT_ERRNO_EQ(EINVAL);
       libc_errno = 0;
 
       ASSERT_EQ(LIBC_NAMESPACE::sched_getparam(-1, &param), -1);
-      ASSERT_EQ(libc_errno, EINVAL);
+      ASSERT_ERRNO_EQ(EINVAL);
       libc_errno = 0;
 
       // Success / missing permissions
@@ -126,14 +126,14 @@ class SchedTest : public LIBC_NAMESPACE::testing::Test {
       libc_errno = 0;
 
       ASSERT_EQ(LIBC_NAMESPACE::sched_getparam(0, &param), 0);
-      ASSERT_EQ(libc_errno, 0);
+      ASSERT_ERRNO_EQ(0);
 
       ASSERT_EQ(param.sched_priority, can_set ? priority : init_priority);
     }
 
     // Null test
     ASSERT_EQ(LIBC_NAMESPACE::sched_setscheduler(0, policy, nullptr), -1);
-    ASSERT_EQ(libc_errno, EINVAL);
+    ASSERT_ERRNO_EQ(EINVAL);
     libc_errno = 0;
   }
 };
@@ -155,10 +155,10 @@ TEST(LlvmLibcSchedParamAndSchedulerTest, NullParamTest) {
   libc_errno = 0;
 
   ASSERT_EQ(LIBC_NAMESPACE::sched_setparam(0, nullptr), -1);
-  ASSERT_EQ(libc_errno, EINVAL);
+  ASSERT_ERRNO_EQ(EINVAL);
   libc_errno = 0;
 
   ASSERT_EQ(LIBC_NAMESPACE::sched_getparam(0, nullptr), -1);
-  ASSERT_EQ(libc_errno, EINVAL);
+  ASSERT_ERRNO_EQ(EINVAL);
   libc_errno = 0;
 }
diff --git a/libc/test/src/sched/sched_rr_get_interval_test.cpp b/libc/test/src/sched/sched_rr_get_interval_test.cpp
index 100100079504165..85a9dbd29bfb6fa 100644
--- a/libc/test/src/sched/sched_rr_get_interval_test.cpp
+++ b/libc/test/src/sched/sched_rr_get_interval_test.cpp
@@ -21,11 +21,11 @@ TEST(LlvmLibcSchedRRGetIntervalTest, SmokeTest) {
   auto SetSched = [&](int policy) {
     int min_priority = LIBC_NAMESPACE::sched_get_priority_min(policy);
     ASSERT_GE(min_priority, 0);
-    ASSERT_EQ(libc_errno, 0);
+    ASSERT_ERRNO_EQ(0);
     struct sched_param param...
[truncated]

#define EXPECT_ERRNO_EQ(VAL) EXPECT_EQ(VAL, static_cast<int>(libc_errno))
#define ASSERT_ERRNO_EQ(VAL) ASSERT_EQ(VAL, static_cast<int>(libc_errno))

#define EXPECT_ERRNO() EXPECT_NE(static_cast<int>(libc_errno), 0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we consistently choose one between ASSERT_ERRNO_EQ(0) and ASSERT_ERRNO()? I see that both are used below. Maybe just pick the former one, and drop ASSERT_ERRNO() form, since the former one is essential for other value.

Copy link
Contributor Author

@gchatelet gchatelet Jan 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ASSERT_ERRNO asserts that the error is not 0 (i.e., there is an error).

To be honest I was not super happy with the naming. Maybe we should have ASSERT_ERRNO_EQ and ASSERT_ERRNO_NE, it would be clearer I think.

Also maybe we don't need the EXPECT and ASSERT versions? How about reducing all of these to just ASSERT_ERRNO_EQ and ASSERT_ERRNO_NE?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or maybe ASSERT_ERRNO_EQ and ASSERT_ERRNO_FAILURE?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed a a new version, let me know what you think.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added another variant with ASSERT_ERRNO_EQ, ASSERT_ERRNO_FAILURE and ASSERT_ERRNO_SUCCESS.

@lntue
Copy link
Contributor

lntue commented Jan 26, 2024

Thanks for cleaning this up! Just a little nit on the style consistency.

@gchatelet gchatelet requested a review from lntue January 26, 2024 14:02
@gchatelet gchatelet merged commit 73874f7 into llvm:main Jan 26, 2024
4 checks passed
@gchatelet gchatelet deleted the add_EXPECT_ERRNO branch January 26, 2024 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants