[libc-commits] [libc] [libc] Fix suseconds_t definition and utimes_test (PR #134326)

Michael Jones via libc-commits libc-commits at lists.llvm.org
Fri Apr 4 12:44:35 PDT 2025


https://github.com/michaelrj-google updated https://github.com/llvm/llvm-project/pull/134326

>From 79bd70d5f9cd3eb2e467852a5ffa4856690e8f73 Mon Sep 17 00:00:00 2001
From: Michael Jones <michaelrj at google.com>
Date: Thu, 3 Apr 2025 16:59:20 -0700
Subject: [PATCH 1/3] [libc] Fix utimes use of file paths

Forgot to use TEST_FILE instead of FILE_PATH in some places.
---
 libc/test/src/sys/time/utimes_test.cpp | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/libc/test/src/sys/time/utimes_test.cpp b/libc/test/src/sys/time/utimes_test.cpp
index 69607ba928e1e..802900e8be7fc 100644
--- a/libc/test/src/sys/time/utimes_test.cpp
+++ b/libc/test/src/sys/time/utimes_test.cpp
@@ -36,11 +36,11 @@ TEST(LlvmLibcUtimesTest, ChangeTimesSpecific) {
   times[1].tv_usec = 23456;
 
   // ensure utimes succeeds
-  ASSERT_THAT(LIBC_NAMESPACE::utimes(FILE_PATH, times), Succeeds(0));
+  ASSERT_THAT(LIBC_NAMESPACE::utimes(TEST_FILE, times), Succeeds(0));
 
   // verify the times values against stat of the TEST_FILE
   struct stat statbuf;
-  ASSERT_EQ(LIBC_NAMESPACE::stat(FILE_PATH, &statbuf), 0);
+  ASSERT_EQ(LIBC_NAMESPACE::stat(TEST_FILE, &statbuf), 0);
 
   // seconds
   ASSERT_EQ(statbuf.st_atim.tv_sec, times[0].tv_sec);
@@ -76,7 +76,7 @@ TEST(LlvmLibcUtimesTest, InvalidMicroseconds) {
   times[1].tv_usec = 1000000; // invalid
 
   // ensure utimes fails
-  ASSERT_THAT(LIBC_NAMESPACE::utimes(FILE_PATH, times), Fails(EINVAL));
+  ASSERT_THAT(LIBC_NAMESPACE::utimes(TEST_FILE, times), Fails(EINVAL));
 
   // check for failure on
   // the other possible bad values
@@ -87,6 +87,6 @@ TEST(LlvmLibcUtimesTest, InvalidMicroseconds) {
   times[1].tv_usec = 1000;
 
   // ensure utimes fails once more
-  ASSERT_THAT(LIBC_NAMESPACE::utimes(FILE_PATH, times), Fails(EINVAL));
+  ASSERT_THAT(LIBC_NAMESPACE::utimes(TEST_FILE, times), Fails(EINVAL));
   ASSERT_THAT(LIBC_NAMESPACE::remove(TEST_FILE), Succeeds(0));
 }

>From bc73e941bc729d43047c0e3a1a9ac4e4a976c48c Mon Sep 17 00:00:00 2001
From: Michael Jones <michaelrj at google.com>
Date: Fri, 4 Apr 2025 11:27:16 -0700
Subject: [PATCH 2/3] fix the actual underlying issue.

---
 libc/include/llvm-libc-types/suseconds_t.h | 7 ++++++-
 libc/test/src/sys/time/utimes_test.cpp     | 7 +++++--
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/libc/include/llvm-libc-types/suseconds_t.h b/libc/include/llvm-libc-types/suseconds_t.h
index 32ecc9f537d00..8e926e8401f5c 100644
--- a/libc/include/llvm-libc-types/suseconds_t.h
+++ b/libc/include/llvm-libc-types/suseconds_t.h
@@ -9,6 +9,11 @@
 #ifndef LLVM_LIBC_TYPES_SUSECONDS_T_H
 #define LLVM_LIBC_TYPES_SUSECONDS_T_H
 
-typedef __INT32_TYPE__ suseconds_t;
+// Per posix: suseconds_t shall be a signed integer type capable of storing
+// values at least in the range [-1, 1000000]. [...] the widths of [other
+// types...] and suseconds_t are no greater than the width of type long.
+
+// The kernel expects 64 bit suseconds_t at least on x86_64.
+typedef long suseconds_t;
 
 #endif // LLVM_LIBC_TYPES_SUSECONDS_T_H
diff --git a/libc/test/src/sys/time/utimes_test.cpp b/libc/test/src/sys/time/utimes_test.cpp
index 802900e8be7fc..6d9b4e3ed2bfc 100644
--- a/libc/test/src/sys/time/utimes_test.cpp
+++ b/libc/test/src/sys/time/utimes_test.cpp
@@ -17,6 +17,8 @@
 #include "test/UnitTest/ErrnoSetterMatcher.h"
 #include "test/UnitTest/Test.h"
 
+#include <sys/stat.h>
+
 // SUCCESS: Takes a file and successfully updates
 // its last access and modified times.
 TEST(LlvmLibcUtimesTest, ChangeTimesSpecific) {
@@ -24,7 +26,8 @@ TEST(LlvmLibcUtimesTest, ChangeTimesSpecific) {
 
   constexpr const char *FILE_PATH = "utimes_pass.test";
   auto TEST_FILE = libc_make_test_file_path(FILE_PATH);
-  int fd = LIBC_NAMESPACE::open(TEST_FILE, O_WRONLY | O_CREAT);
+  int fd = LIBC_NAMESPACE::open(TEST_FILE, O_WRONLY | O_CREAT, S_IRWXU);
+  ASSERT_ERRNO_SUCCESS();
   ASSERT_GT(fd, 0);
   ASSERT_THAT(LIBC_NAMESPACE::close(fd), Succeeds(0));
 
@@ -63,7 +66,7 @@ TEST(LlvmLibcUtimesTest, InvalidMicroseconds) {
 
   constexpr const char *FILE_PATH = "utimes_fail.test";
   auto TEST_FILE = libc_make_test_file_path(FILE_PATH);
-  int fd = LIBC_NAMESPACE::open(TEST_FILE, O_WRONLY | O_CREAT);
+  int fd = LIBC_NAMESPACE::open(TEST_FILE, O_WRONLY | O_CREAT, S_IRWXU);
   ASSERT_GT(fd, 0);
   ASSERT_THAT(LIBC_NAMESPACE::close(fd), Succeeds(0));
 

>From 4c57378e6ab68bf8464a778b1ff8114bb8211935 Mon Sep 17 00:00:00 2001
From: Michael Jones <michaelrj at google.com>
Date: Fri, 4 Apr 2025 12:44:00 -0700
Subject: [PATCH 3/3] use proxy header, new errno test framework.

---
 libc/test/src/sys/time/CMakeLists.txt  | 2 ++
 libc/test/src/sys/time/utimes_test.cpp | 9 ++++++---
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/libc/test/src/sys/time/CMakeLists.txt b/libc/test/src/sys/time/CMakeLists.txt
index c092d33e43d85..72a65eec00937 100644
--- a/libc/test/src/sys/time/CMakeLists.txt
+++ b/libc/test/src/sys/time/CMakeLists.txt
@@ -8,6 +8,7 @@ add_libc_unittest(
     utimes_test.cpp
   DEPENDS
     libc.hdr.fcntl_macros
+    libc.hdr.sys_stat_macros
     libc.src.errno.errno
     libc.src.fcntl.open
     libc.src.sys.time.utimes
@@ -16,4 +17,5 @@ add_libc_unittest(
     libc.src.unistd.write
     libc.src.stdio.remove
     libc.src.sys.stat.stat
+    libc.test.UnitTest.ErrnoCheckingTest
 )
diff --git a/libc/test/src/sys/time/utimes_test.cpp b/libc/test/src/sys/time/utimes_test.cpp
index 6d9b4e3ed2bfc..66e69a1b2a700 100644
--- a/libc/test/src/sys/time/utimes_test.cpp
+++ b/libc/test/src/sys/time/utimes_test.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "hdr/fcntl_macros.h"
+#include "hdr/sys_stat_macros.h"
 #include "hdr/types/struct_timeval.h"
 #include "src/errno/libc_errno.h"
 #include "src/fcntl/open.h"
@@ -14,14 +15,16 @@
 #include "src/sys/stat/stat.h"
 #include "src/sys/time/utimes.h"
 #include "src/unistd/close.h"
+
+#include "test/UnitTest/ErrnoCheckingTest.h"
 #include "test/UnitTest/ErrnoSetterMatcher.h"
 #include "test/UnitTest/Test.h"
 
-#include <sys/stat.h>
+using LlvmLibcUtimesTest = LIBC_NAMESPACE::testing::ErrnoCheckingTest;
 
 // SUCCESS: Takes a file and successfully updates
 // its last access and modified times.
-TEST(LlvmLibcUtimesTest, ChangeTimesSpecific) {
+TEST_F(LlvmLibcUtimesTest, ChangeTimesSpecific) {
   using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds;
 
   constexpr const char *FILE_PATH = "utimes_pass.test";
@@ -60,7 +63,7 @@ TEST(LlvmLibcUtimesTest, ChangeTimesSpecific) {
 
 // FAILURE: Invalid values in the timeval struct
 // to check that utimes rejects it.
-TEST(LlvmLibcUtimesTest, InvalidMicroseconds) {
+TEST_F(LlvmLibcUtimesTest, InvalidMicroseconds) {
   using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Fails;
   using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds;
 



More information about the libc-commits mailing list