summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
Diffstat
-rw-r--r--pkg/sentry/fs/fsutil/inode.go4
-rw-r--r--pkg/sentry/fs/fsutil/inode_cached.go8
-rw-r--r--pkg/sentry/fs/gofer/path.go7
-rw-r--r--pkg/sentry/fs/host/inode.go2
-rw-r--r--pkg/sentry/fs/inode.go2
-rw-r--r--pkg/sentry/fs/inode_operations.go5
-rw-r--r--pkg/sentry/fs/inode_overlay.go2
-rw-r--r--pkg/sentry/fs/mock.go2
-rw-r--r--pkg/sentry/fs/ramfs/dir.go2
-rw-r--r--pkg/sentry/fs/tmpfs/inode_file.go2
-rw-r--r--pkg/sentry/fs/tmpfs/tmpfs.go8
-rw-r--r--test/syscalls/linux/stat_times.cc319
12 files changed, 213 insertions, 150 deletions
diff --git a/pkg/sentry/fs/fsutil/inode.go b/pkg/sentry/fs/fsutil/inode.go
index 151be1d0d..5e1bfeb58 100644
--- a/pkg/sentry/fs/fsutil/inode.go
+++ b/pkg/sentry/fs/fsutil/inode.go
@@ -341,7 +341,7 @@ func (InodeNotDirectory) RemoveDirectory(context.Context, *fs.Inode, string) err
}
// Rename implements fs.FileOperations.Rename.
-func (InodeNotDirectory) Rename(context.Context, *fs.Inode, string, *fs.Inode, string, bool) error {
+func (InodeNotDirectory) Rename(context.Context, *fs.Inode, *fs.Inode, string, *fs.Inode, string, bool) error {
return syserror.EINVAL
}
@@ -381,7 +381,7 @@ func (InodeNoopTruncate) Truncate(context.Context, *fs.Inode, int64) error {
type InodeNotRenameable struct{}
// Rename implements fs.InodeOperations.Rename.
-func (InodeNotRenameable) Rename(context.Context, *fs.Inode, string, *fs.Inode, string, bool) error {
+func (InodeNotRenameable) Rename(context.Context, *fs.Inode, *fs.Inode, string, *fs.Inode, string, bool) error {
return syserror.EINVAL
}
diff --git a/pkg/sentry/fs/fsutil/inode_cached.go b/pkg/sentry/fs/fsutil/inode_cached.go
index 03cad37f3..bc0b8c744 100644
--- a/pkg/sentry/fs/fsutil/inode_cached.go
+++ b/pkg/sentry/fs/fsutil/inode_cached.go
@@ -451,6 +451,14 @@ func (c *CachingInodeOperations) touchModificationTimeLocked(now time.Time) {
c.dirtyAttr.StatusChangeTime = true
}
+// TouchStatusChangeTime updates the cached status change time in-place to the
+// current time.
+func (c *CachingInodeOperations) TouchStatusChangeTime(ctx context.Context) {
+ c.attrMu.Lock()
+ c.touchStatusChangeTimeLocked(ktime.NowFromContext(ctx))
+ c.attrMu.Unlock()
+}
+
// touchStatusChangeTimeLocked updates the cached status change time
// in-place to the current time.
//
diff --git a/pkg/sentry/fs/gofer/path.go b/pkg/sentry/fs/gofer/path.go
index 875de8b5f..babfa4560 100644
--- a/pkg/sentry/fs/gofer/path.go
+++ b/pkg/sentry/fs/gofer/path.go
@@ -344,7 +344,7 @@ func (i *inodeOperations) RemoveDirectory(ctx context.Context, dir *fs.Inode, na
}
// Rename renames this node.
-func (i *inodeOperations) Rename(ctx context.Context, oldParent *fs.Inode, oldName string, newParent *fs.Inode, newName string, replacement bool) error {
+func (i *inodeOperations) Rename(ctx context.Context, inode *fs.Inode, oldParent *fs.Inode, oldName string, newParent *fs.Inode, newName string, replacement bool) error {
if len(newName) > maxFilenameLen {
return syserror.ENAMETOOLONG
}
@@ -389,6 +389,11 @@ func (i *inodeOperations) Rename(ctx context.Context, oldParent *fs.Inode, oldNa
newParentInodeOperations.markDirectoryDirty()
}
}
+
+ // Rename always updates ctime.
+ if i.session().cachePolicy.cacheUAttrs(inode) {
+ i.cachingInodeOps.TouchStatusChangeTime(ctx)
+ }
return nil
}
diff --git a/pkg/sentry/fs/host/inode.go b/pkg/sentry/fs/host/inode.go
index d36ac9a87..ebf2154bc 100644
--- a/pkg/sentry/fs/host/inode.go
+++ b/pkg/sentry/fs/host/inode.go
@@ -301,7 +301,7 @@ func (i *inodeOperations) RemoveDirectory(ctx context.Context, dir *fs.Inode, na
}
// Rename implements fs.InodeOperations.Rename.
-func (i *inodeOperations) Rename(ctx context.Context, oldParent *fs.Inode, oldName string, newParent *fs.Inode, newName string, replacement bool) error {
+func (i *inodeOperations) Rename(ctx context.Context, inode *fs.Inode, oldParent *fs.Inode, oldName string, newParent *fs.Inode, newName string, replacement bool) error {
op, ok := oldParent.InodeOperations.(*inodeOperations)
if !ok {
return syscall.EXDEV
diff --git a/pkg/sentry/fs/inode.go b/pkg/sentry/fs/inode.go
index 22f316daf..aef1a1cb9 100644
--- a/pkg/sentry/fs/inode.go
+++ b/pkg/sentry/fs/inode.go
@@ -216,7 +216,7 @@ func (i *Inode) Rename(ctx context.Context, oldParent *Dirent, renamed *Dirent,
if i.overlay != nil {
return overlayRename(ctx, i.overlay, oldParent, renamed, newParent, newName, replacement)
}
- return i.InodeOperations.Rename(ctx, oldParent.Inode, renamed.name, newParent.Inode, newName, replacement)
+ return i.InodeOperations.Rename(ctx, renamed.Inode, oldParent.Inode, renamed.name, newParent.Inode, newName, replacement)
}
// Bind calls i.InodeOperations.Bind with i as the directory.
diff --git a/pkg/sentry/fs/inode_operations.go b/pkg/sentry/fs/inode_operations.go
index abafe4791..3211f1817 100644
--- a/pkg/sentry/fs/inode_operations.go
+++ b/pkg/sentry/fs/inode_operations.go
@@ -131,14 +131,15 @@ type InodeOperations interface {
RemoveDirectory(ctx context.Context, dir *Inode, name string) error
// Rename atomically renames oldName under oldParent to newName under
- // newParent where oldParent and newParent are directories.
+ // newParent where oldParent and newParent are directories. inode is
+ // the Inode of this InodeOperations.
//
// If replacement is true, then newName already exists and this call
// will replace it with oldName.
//
// Implementations are responsible for rejecting renames that replace
// non-empty directories.
- Rename(ctx context.Context, oldParent *Inode, oldName string, newParent *Inode, newName string, replacement bool) error
+ Rename(ctx context.Context, inode *Inode, oldParent *Inode, oldName string, newParent *Inode, newName string, replacement bool) error
// Bind binds a new socket under dir at the given name.
//
diff --git a/pkg/sentry/fs/inode_overlay.go b/pkg/sentry/fs/inode_overlay.go
index ead487097..ea574224f 100644
--- a/pkg/sentry/fs/inode_overlay.go
+++ b/pkg/sentry/fs/inode_overlay.go
@@ -389,7 +389,7 @@ func overlayRename(ctx context.Context, o *overlayEntry, oldParent *Dirent, rena
return err
}
oldName := renamed.name
- if err := o.upper.InodeOperations.Rename(ctx, oldParent.Inode.overlay.upper, oldName, newParent.Inode.overlay.upper, newName, replacement); err != nil {
+ if err := o.upper.InodeOperations.Rename(ctx, renamed.Inode.overlay.upper, oldParent.Inode.overlay.upper, oldName, newParent.Inode.overlay.upper, newName, replacement); err != nil {
return err
}
if renamed.Inode.overlay.lowerExists {
diff --git a/pkg/sentry/fs/mock.go b/pkg/sentry/fs/mock.go
index a71144b2c..064943c5b 100644
--- a/pkg/sentry/fs/mock.go
+++ b/pkg/sentry/fs/mock.go
@@ -132,7 +132,7 @@ func (n *MockInodeOperations) CreateDirectory(context.Context, *Inode, string, F
}
// Rename implements fs.InodeOperations.Rename.
-func (n *MockInodeOperations) Rename(ctx context.Context, oldParent *Inode, oldName string, newParent *Inode, newName string, replacement bool) error {
+func (n *MockInodeOperations) Rename(ctx context.Context, inode *Inode, oldParent *Inode, oldName string, newParent *Inode, newName string, replacement bool) error {
n.renameCalled = true
return nil
}
diff --git a/pkg/sentry/fs/ramfs/dir.go b/pkg/sentry/fs/ramfs/dir.go
index eb98b59cc..c97ad26f5 100644
--- a/pkg/sentry/fs/ramfs/dir.go
+++ b/pkg/sentry/fs/ramfs/dir.go
@@ -401,7 +401,7 @@ func (d *Dir) GetFile(ctx context.Context, dirent *fs.Dirent, flags fs.FileFlags
}
// Rename implements fs.InodeOperations.Rename.
-func (*Dir) Rename(ctx context.Context, oldParent *fs.Inode, oldName string, newParent *fs.Inode, newName string, replacement bool) error {
+func (*Dir) Rename(ctx context.Context, inode *fs.Inode, oldParent *fs.Inode, oldName string, newParent *fs.Inode, newName string, replacement bool) error {
return Rename(ctx, oldParent.InodeOperations, oldName, newParent.InodeOperations, newName, replacement)
}
diff --git a/pkg/sentry/fs/tmpfs/inode_file.go b/pkg/sentry/fs/tmpfs/inode_file.go
index c90062a22..3fe659543 100644
--- a/pkg/sentry/fs/tmpfs/inode_file.go
+++ b/pkg/sentry/fs/tmpfs/inode_file.go
@@ -149,7 +149,7 @@ func (f *fileInodeOperations) Mappable(*fs.Inode) memmap.Mappable {
}
// Rename implements fs.InodeOperations.Rename.
-func (*fileInodeOperations) Rename(ctx context.Context, oldParent *fs.Inode, oldName string, newParent *fs.Inode, newName string, replacement bool) error {
+func (*fileInodeOperations) Rename(ctx context.Context, inode *fs.Inode, oldParent *fs.Inode, oldName string, newParent *fs.Inode, newName string, replacement bool) error {
return rename(ctx, oldParent, oldName, newParent, newName, replacement)
}
diff --git a/pkg/sentry/fs/tmpfs/tmpfs.go b/pkg/sentry/fs/tmpfs/tmpfs.go
index 6ad5c5adb..263d10cfe 100644
--- a/pkg/sentry/fs/tmpfs/tmpfs.go
+++ b/pkg/sentry/fs/tmpfs/tmpfs.go
@@ -238,7 +238,7 @@ func (d *Dir) newCreateOps() *ramfs.CreateOps {
}
// Rename implements fs.InodeOperations.Rename.
-func (d *Dir) Rename(ctx context.Context, oldParent *fs.Inode, oldName string, newParent *fs.Inode, newName string, replacement bool) error {
+func (d *Dir) Rename(ctx context.Context, inode *fs.Inode, oldParent *fs.Inode, oldName string, newParent *fs.Inode, newName string, replacement bool) error {
return rename(ctx, oldParent, oldName, newParent, newName, replacement)
}
@@ -271,7 +271,7 @@ func NewSymlink(ctx context.Context, target string, owner fs.FileOwner, msrc *fs
}
// Rename implements fs.InodeOperations.Rename.
-func (s *Symlink) Rename(ctx context.Context, oldParent *fs.Inode, oldName string, newParent *fs.Inode, newName string, replacement bool) error {
+func (s *Symlink) Rename(ctx context.Context, inode *fs.Inode, oldParent *fs.Inode, oldName string, newParent *fs.Inode, newName string, replacement bool) error {
return rename(ctx, oldParent, oldName, newParent, newName, replacement)
}
@@ -301,7 +301,7 @@ func NewSocket(ctx context.Context, socket transport.BoundEndpoint, owner fs.Fil
}
// Rename implements fs.InodeOperations.Rename.
-func (s *Socket) Rename(ctx context.Context, oldParent *fs.Inode, oldName string, newParent *fs.Inode, newName string, replacement bool) error {
+func (s *Socket) Rename(ctx context.Context, inode *fs.Inode, oldParent *fs.Inode, oldName string, newParent *fs.Inode, newName string, replacement bool) error {
return rename(ctx, oldParent, oldName, newParent, newName, replacement)
}
@@ -338,7 +338,7 @@ func NewFifo(ctx context.Context, owner fs.FileOwner, perms fs.FilePermissions,
}
// Rename implements fs.InodeOperations.Rename.
-func (f *Fifo) Rename(ctx context.Context, oldParent *fs.Inode, oldName string, newParent *fs.Inode, newName string, replacement bool) error {
+func (f *Fifo) Rename(ctx context.Context, inode *fs.Inode, oldParent *fs.Inode, oldName string, newParent *fs.Inode, newName string, replacement bool) error {
return rename(ctx, oldParent, oldName, newParent, newName, replacement)
}
diff --git a/test/syscalls/linux/stat_times.cc b/test/syscalls/linux/stat_times.cc
index 9b53739a0..195c87ca5 100644
--- a/test/syscalls/linux/stat_times.cc
+++ b/test/syscalls/linux/stat_times.cc
@@ -33,23 +33,88 @@ namespace {
using ::testing::IsEmpty;
using ::testing::Not;
-class StatTimesTest : public ::testing::Test {
- protected:
- std::tuple<absl::Time, absl::Time, absl::Time> GetTime(const TempPath& file) {
- struct stat statbuf = {};
- EXPECT_THAT(stat(file.path().c_str(), &statbuf), SyscallSucceeds());
-
- const auto atime = absl::TimeFromTimespec(statbuf.st_atim);
- const auto mtime = absl::TimeFromTimespec(statbuf.st_mtim);
- const auto ctime = absl::TimeFromTimespec(statbuf.st_ctim);
- return std::make_tuple(atime, mtime, ctime);
- }
+std::tuple<absl::Time, absl::Time, absl::Time> GetTime(const TempPath& file) {
+ struct stat statbuf = {};
+ EXPECT_THAT(stat(file.path().c_str(), &statbuf), SyscallSucceeds());
+
+ const auto atime = absl::TimeFromTimespec(statbuf.st_atim);
+ const auto mtime = absl::TimeFromTimespec(statbuf.st_mtim);
+ const auto ctime = absl::TimeFromTimespec(statbuf.st_ctim);
+ return std::make_tuple(atime, mtime, ctime);
+}
+
+enum class AtimeEffect {
+ Unchanged,
+ Changed,
+};
+
+enum class MtimeEffect {
+ Unchanged,
+ Changed,
};
-TEST_F(StatTimesTest, FileCreationTimes) {
+enum class CtimeEffect {
+ Unchanged,
+ Changed,
+};
+
+// Tests that fn modifies the atime/mtime/ctime of path as specified.
+void CheckTimes(const TempPath& path, std::function<void()> fn,
+ AtimeEffect atime_effect, MtimeEffect mtime_effect,
+ CtimeEffect ctime_effect) {
+ absl::Time atime, mtime, ctime;
+ std::tie(atime, mtime, ctime) = GetTime(path);
+
+ // FIXME(b/132819225): gVisor filesystem timestamps inconsistently use the
+ // internal or host clock, which may diverge slightly. Allow some slack on
+ // times to account for the difference.
+ //
+ // Here we sleep for 1s so that initial creation of path doesn't fall within
+ // the before slack window.
+ absl::SleepFor(absl::Seconds(1));
+
+ const absl::Time before = absl::Now() - absl::Seconds(1);
+
+ // Perform the op.
+ fn();
+
+ const absl::Time after = absl::Now() + absl::Seconds(1);
+
+ absl::Time atime2, mtime2, ctime2;
+ std::tie(atime2, mtime2, ctime2) = GetTime(path);
+
+ if (atime_effect == AtimeEffect::Changed) {
+ EXPECT_LE(before, atime2);
+ EXPECT_GE(after, atime2);
+ EXPECT_GT(atime2, atime);
+ } else {
+ EXPECT_EQ(atime2, atime);
+ }
+
+ if (mtime_effect == MtimeEffect::Changed) {
+ EXPECT_LE(before, mtime2);
+ EXPECT_GE(after, mtime2);
+ EXPECT_GT(mtime2, mtime);
+ } else {
+ EXPECT_EQ(mtime2, mtime);
+ }
+
+ if (ctime_effect == CtimeEffect::Changed) {
+ EXPECT_LE(before, ctime2);
+ EXPECT_GE(after, ctime2);
+ EXPECT_GT(ctime2, ctime);
+ } else {
+ EXPECT_EQ(ctime2, ctime);
+ }
+}
+
+// File creation time is reflected in atime, mtime, and ctime.
+TEST(StatTimesTest, FileCreation) {
const DisableSave ds; // Timing-related test.
// Get a time for when the file is created.
+ //
+ // FIXME(b/132819225): See above.
const absl::Time before = absl::Now() - absl::Seconds(1);
const TempPath file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile());
const absl::Time after = absl::Now() + absl::Seconds(1);
@@ -65,153 +130,137 @@ TEST_F(StatTimesTest, FileCreationTimes) {
EXPECT_GE(after, ctime);
}
-TEST_F(StatTimesTest, FileCtimeChanges) {
- auto file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile());
-
- MaybeSave(); // FIXME(b/69865927): ctime is inconsistent.
-
- absl::Time atime, mtime, ctime;
- std::tie(atime, mtime, ctime) = GetTime(file);
-
- absl::SleepFor(absl::Seconds(1));
-
- // Chmod should only change ctime.
- EXPECT_THAT(chmod(file.path().c_str(), 0666), SyscallSucceeds());
+// Calling chmod on a file changes ctime.
+TEST(StatTimesTest, FileChmod) {
+ TempPath file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile());
- absl::Time atime2, mtime2, ctime2;
- std::tie(atime2, mtime2, ctime2) = GetTime(file);
- EXPECT_EQ(atime2, atime);
- EXPECT_EQ(mtime2, mtime);
- EXPECT_GT(ctime2, ctime);
+ auto fn = [&] {
+ EXPECT_THAT(chmod(file.path().c_str(), 0666), SyscallSucceeds());
+ };
+ CheckTimes(file, fn, AtimeEffect::Unchanged, MtimeEffect::Unchanged,
+ CtimeEffect::Changed);
+}
- absl::SleepFor(absl::Seconds(1));
+// Renaming a file changes ctime.
+TEST(StatTimesTest, FileRename) {
+ TempPath file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile());
- // Rename should only change ctime.
- const auto newpath = NewTempAbsPath();
- EXPECT_THAT(rename(file.path().c_str(), newpath.c_str()), SyscallSucceeds());
- file.reset(newpath);
+ const std::string newpath = NewTempAbsPath();
- std::tie(atime, mtime, ctime) = GetTime(file);
- EXPECT_EQ(atime, atime2);
- EXPECT_EQ(mtime, mtime2);
- EXPECT_GT(ctime, ctime2);
+ auto fn = [&] {
+ ASSERT_THAT(rename(file.release().c_str(), newpath.c_str()),
+ SyscallSucceeds());
+ file.reset(newpath);
+ };
+ CheckTimes(file, fn, AtimeEffect::Unchanged, MtimeEffect::Unchanged,
+ CtimeEffect::Changed);
+}
- absl::SleepFor(absl::Seconds(1));
+// Renaming a file changes ctime, even with an open FD.
+//
+// NOTE(b/132732387): This is a regression test for fs/gofer failing to update
+// cached ctime.
+TEST(StatTimesTest, FileRenameOpenFD) {
+ TempPath file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile());
+
+ // Holding an FD shouldn't affect behavior.
+ const FileDescriptor fd =
+ ASSERT_NO_ERRNO_AND_VALUE(Open(file.path(), O_RDONLY));
+
+ const std::string newpath = NewTempAbsPath();
+
+ // FIXME(b/132814682): Restore fails with an uncached gofer and an open FD
+ // across rename.
+ //
+ // N.B. The logic here looks backwards because it isn't possible to
+ // conditionally disable save, only conditionally re-enable it.
+ DisableSave ds;
+ if (!getenv("GVISOR_GOFER_UNCACHED")) {
+ ds.reset();
+ }
- // Utimes should only change ctime and the time that we ask to change (atime
- // to now in this case).
- const absl::Time before = absl::Now() - absl::Seconds(1);
- const struct timespec ts[2] = {{0, UTIME_NOW}, {0, UTIME_OMIT}};
- ASSERT_THAT(utimensat(AT_FDCWD, file.path().c_str(), ts, 0),
- SyscallSucceeds());
- const absl::Time after = absl::Now() + absl::Seconds(1);
+ auto fn = [&] {
+ ASSERT_THAT(rename(file.release().c_str(), newpath.c_str()),
+ SyscallSucceeds());
+ file.reset(newpath);
+ };
+ CheckTimes(file, fn, AtimeEffect::Unchanged, MtimeEffect::Unchanged,
+ CtimeEffect::Changed);
+}
- std::tie(atime2, mtime2, ctime2) = GetTime(file);
- EXPECT_LE(before, atime2);
- EXPECT_GE(after, atime2);
- EXPECT_EQ(mtime2, mtime);
- EXPECT_GT(ctime2, ctime);
+// Calling utimes on a file changes ctime and the time that we ask to change
+// (atime to now in this case).
+TEST(StatTimesTest, FileUtimes) {
+ TempPath file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile());
+
+ auto fn = [&] {
+ const struct timespec ts[2] = {{0, UTIME_NOW}, {0, UTIME_OMIT}};
+ ASSERT_THAT(utimensat(AT_FDCWD, file.path().c_str(), ts, 0),
+ SyscallSucceeds());
+ };
+ CheckTimes(file, fn, AtimeEffect::Changed, MtimeEffect::Unchanged,
+ CtimeEffect::Changed);
}
-TEST_F(StatTimesTest, FileMtimeChanges) {
- const auto file = ASSERT_NO_ERRNO_AND_VALUE(
+// Truncating a file changes mtime and ctime.
+TEST(StatTimesTest, FileTruncate) {
+ const TempPath file = ASSERT_NO_ERRNO_AND_VALUE(
TempPath::CreateFileWith(GetAbsoluteTestTmpdir(), "yaaass", 0666));
- absl::Time atime, mtime, ctime;
- std::tie(atime, mtime, ctime) = GetTime(file);
-
- absl::SleepFor(absl::Seconds(1));
-
- // Truncate should only change mtime and ctime.
- EXPECT_THAT(truncate(file.path().c_str(), 0), SyscallSucceeds());
-
- absl::Time atime2, mtime2, ctime2;
- std::tie(atime2, mtime2, ctime2) = GetTime(file);
- EXPECT_EQ(atime2, atime);
- EXPECT_GT(mtime2, mtime);
- EXPECT_GT(ctime2, ctime);
+ auto fn = [&] {
+ EXPECT_THAT(truncate(file.path().c_str(), 0), SyscallSucceeds());
+ };
+ CheckTimes(file, fn, AtimeEffect::Unchanged, MtimeEffect::Changed,
+ CtimeEffect::Changed);
+}
- absl::SleepFor(absl::Seconds(1));
+// Writing a file changes mtime and ctime.
+TEST(StatTimesTest, FileWrite) {
+ const TempPath file = ASSERT_NO_ERRNO_AND_VALUE(
+ TempPath::CreateFileWith(GetAbsoluteTestTmpdir(), "yaaass", 0666));
- // Write should only change mtime and ctime.
- const auto fd = ASSERT_NO_ERRNO_AND_VALUE(Open(file.path(), O_RDWR, 0));
- const std::string contents = "all the single dollars";
- EXPECT_THAT(write(fd.get(), contents.data(), contents.size()),
- SyscallSucceeds());
+ const FileDescriptor fd =
+ ASSERT_NO_ERRNO_AND_VALUE(Open(file.path(), O_RDWR, 0));
- std::tie(atime, mtime, ctime) = GetTime(file);
- EXPECT_EQ(atime, atime2);
- EXPECT_GT(mtime, mtime2);
- EXPECT_GT(ctime, ctime2);
+ auto fn = [&] {
+ const std::string contents = "all the single dollars";
+ EXPECT_THAT(WriteFd(fd.get(), contents.data(), contents.size()),
+ SyscallSucceeds());
+ };
+ CheckTimes(file, fn, AtimeEffect::Unchanged, MtimeEffect::Changed,
+ CtimeEffect::Changed);
}
-TEST_F(StatTimesTest, FileAtimeChanges) {
+// Reading a file changes atime.
+TEST(StatTimesTest, FileRead) {
const std::string contents = "bills bills bills";
- const auto file = ASSERT_NO_ERRNO_AND_VALUE(
+ const TempPath file = ASSERT_NO_ERRNO_AND_VALUE(
TempPath::CreateFileWith(GetAbsoluteTestTmpdir(), contents, 0666));
- MaybeSave(); // FIXME(b/69865927): ctime is inconsistent.
-
- absl::Time atime, mtime, ctime;
- std::tie(atime, mtime, ctime) = GetTime(file);
-
- absl::SleepFor(absl::Seconds(1));
-
- const auto fd = ASSERT_NO_ERRNO_AND_VALUE(Open(file.path(), O_RDONLY, 0));
+ const FileDescriptor fd =
+ ASSERT_NO_ERRNO_AND_VALUE(Open(file.path(), O_RDONLY, 0));
- // Read should only change atime.
- char buf[20];
- const absl::Time before = absl::Now() - absl::Seconds(1);
- int read_result;
- ASSERT_THAT(read_result = read(fd.get(), buf, sizeof(buf)),
- SyscallSucceeds());
- const absl::Time after = absl::Now() + absl::Seconds(1);
-
- EXPECT_EQ(std::string(buf, read_result), contents);
-
- absl::Time atime2, mtime2, ctime2;
- std::tie(atime2, mtime2, ctime2) = GetTime(file);
-
- EXPECT_LE(before, atime2);
- EXPECT_GE(after, atime2);
- EXPECT_GT(atime2, atime);
- EXPECT_EQ(mtime2, mtime);
- EXPECT_EQ(ctime2, ctime);
+ auto fn = [&] {
+ char buf[20];
+ ASSERT_THAT(ReadFd(fd.get(), buf, sizeof(buf)),
+ SyscallSucceedsWithValue(contents.size()));
+ };
+ CheckTimes(file, fn, AtimeEffect::Changed, MtimeEffect::Unchanged,
+ CtimeEffect::Unchanged);
}
-TEST_F(StatTimesTest, DirAtimeChanges) {
- const auto dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
- const auto file =
+// Listing files in a directory changes atime.
+TEST(StatTimesTest, DirList) {
+ const TempPath dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
+ const TempPath file =
ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFileIn(dir.path()));
- MaybeSave(); // FIXME(b/69865927): ctime is inconsistent.
-
- absl::Time atime, mtime, ctime;
- std::tie(atime, mtime, ctime) = GetTime(dir);
-
- absl::SleepFor(absl::Seconds(1));
-
- const absl::Time before = absl::Now() - absl::Seconds(1);
-
- // NOTE(b/37756234): Keep an fd open. This ensures that the inode backing the
- // directory won't be destroyed before the final GetTime to avoid writing out
- // timestamps and causing side effects.
- const auto fd = ASSERT_NO_ERRNO_AND_VALUE(Open(dir.path(), O_RDONLY, 0));
-
- // Listing the directory contents should only change atime.
- auto contents = ASSERT_NO_ERRNO_AND_VALUE(ListDir(dir.path(), false));
- EXPECT_THAT(contents, Not(IsEmpty()));
-
- const absl::Time after = absl::Now() + absl::Seconds(1);
-
- absl::Time atime2, mtime2, ctime2;
- std::tie(atime2, mtime2, ctime2) = GetTime(dir);
-
- EXPECT_LE(before, atime2);
- EXPECT_GE(after, atime2);
- EXPECT_GT(atime2, atime);
- EXPECT_EQ(mtime2, mtime);
- EXPECT_EQ(ctime2, ctime);
+ auto fn = [&] {
+ const auto contents = ASSERT_NO_ERRNO_AND_VALUE(ListDir(dir.path(), false));
+ EXPECT_THAT(contents, Not(IsEmpty()));
+ };
+ CheckTimes(dir, fn, AtimeEffect::Changed, MtimeEffect::Unchanged,
+ CtimeEffect::Unchanged);
}
} // namespace
generated by cgit v1.2.3 (git 2.39.1) at 2024-11-18 20:44:46 +0000