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

Fix handling of negative timestamps in Stat. #999

Merged
merged 3 commits into from
Jan 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 14 additions & 36 deletions src/backend/libc/fs/syscalls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1778,6 +1778,7 @@ pub(crate) fn sendfile(

/// Convert from a Linux `statx` value to rustix's `Stat`.
#[cfg(all(linux_kernel, target_pointer_width = "32"))]
#[allow(deprecated)] // for `st_[amc]time` u64->i64 transition
fn statx_to_stat(x: crate::fs::Statx) -> io::Result<Stat> {
Ok(Stat {
st_dev: crate::fs::makedev(x.stx_dev_major, x.stx_dev_minor).into(),
Expand All @@ -1789,23 +1790,11 @@ fn statx_to_stat(x: crate::fs::Statx) -> io::Result<Stat> {
st_size: x.stx_size.try_into().map_err(|_| io::Errno::OVERFLOW)?,
st_blksize: x.stx_blksize.into(),
st_blocks: x.stx_blocks.into(),
st_atime: x
.stx_atime
.tv_sec
.try_into()
.map_err(|_| io::Errno::OVERFLOW)?,
st_atime: bitcast!(i64::from(x.stx_atime.tv_sec)),
st_atime_nsec: x.stx_atime.tv_nsec as _,
st_mtime: x
.stx_mtime
.tv_sec
.try_into()
.map_err(|_| io::Errno::OVERFLOW)?,
st_mtime: bitcast!(i64::from(x.stx_mtime.tv_sec)),
st_mtime_nsec: x.stx_mtime.tv_nsec as _,
st_ctime: x
.stx_ctime
.tv_sec
.try_into()
.map_err(|_| io::Errno::OVERFLOW)?,
st_ctime: bitcast!(i64::from(x.stx_ctime.tv_sec)),
st_ctime_nsec: x.stx_ctime.tv_nsec as _,
st_ino: x.stx_ino.into(),
})
Expand All @@ -1827,23 +1816,11 @@ fn statx_to_stat(x: crate::fs::Statx) -> io::Result<Stat> {
result.st_size = x.stx_size.try_into().map_err(|_| io::Errno::OVERFLOW)?;
result.st_blksize = x.stx_blksize.into();
result.st_blocks = x.stx_blocks.try_into().map_err(|_e| io::Errno::OVERFLOW)?;
result.st_atime = x
.stx_atime
.tv_sec
.try_into()
.map_err(|_| io::Errno::OVERFLOW)?;
result.st_atime = bitcast!(i64::from(x.stx_atime.tv_sec));
result.st_atime_nsec = x.stx_atime.tv_nsec as _;
result.st_mtime = x
.stx_mtime
.tv_sec
.try_into()
.map_err(|_| io::Errno::OVERFLOW)?;
result.st_mtime = bitcast!(i64::from(x.stx_mtime.tv_sec));
result.st_mtime_nsec = x.stx_mtime.tv_nsec as _;
result.st_ctime = x
.stx_ctime
.tv_sec
.try_into()
.map_err(|_| io::Errno::OVERFLOW)?;
result.st_ctime = bitcast!(i64::from(x.stx_ctime.tv_sec));
result.st_ctime_nsec = x.stx_ctime.tv_nsec as _;
result.st_ino = x.stx_ino.into();

Expand All @@ -1852,6 +1829,7 @@ fn statx_to_stat(x: crate::fs::Statx) -> io::Result<Stat> {

/// Convert from a Linux `stat64` value to rustix's `Stat`.
#[cfg(all(linux_kernel, target_pointer_width = "32"))]
#[allow(deprecated)] // for `st_[amc]time` u64->i64 transition
fn stat64_to_stat(s64: c::stat64) -> io::Result<Stat> {
Ok(Stat {
st_dev: s64.st_dev.try_into().map_err(|_| io::Errno::OVERFLOW)?,
Expand All @@ -1863,17 +1841,17 @@ fn stat64_to_stat(s64: c::stat64) -> io::Result<Stat> {
st_size: s64.st_size.try_into().map_err(|_| io::Errno::OVERFLOW)?,
st_blksize: s64.st_blksize.try_into().map_err(|_| io::Errno::OVERFLOW)?,
st_blocks: s64.st_blocks.try_into().map_err(|_| io::Errno::OVERFLOW)?,
st_atime: s64.st_atime.try_into().map_err(|_| io::Errno::OVERFLOW)?,
st_atime: bitcast!(i64::from(s64.st_atime)),
st_atime_nsec: s64
.st_atime_nsec
.try_into()
.map_err(|_| io::Errno::OVERFLOW)?,
st_mtime: s64.st_mtime.try_into().map_err(|_| io::Errno::OVERFLOW)?,
st_mtime: bitcast!(i64::from(s64.st_mtime)),
st_mtime_nsec: s64
.st_mtime_nsec
.try_into()
.map_err(|_| io::Errno::OVERFLOW)?,
st_ctime: s64.st_ctime.try_into().map_err(|_| io::Errno::OVERFLOW)?,
st_ctime: bitcast!(i64::from(s64.st_ctime)),
st_ctime_nsec: s64
.st_ctime_nsec
.try_into()
Expand All @@ -1899,17 +1877,17 @@ fn stat64_to_stat(s64: c::stat64) -> io::Result<Stat> {
result.st_size = s64.st_size.try_into().map_err(|_| io::Errno::OVERFLOW)?;
result.st_blksize = s64.st_blksize.try_into().map_err(|_| io::Errno::OVERFLOW)?;
result.st_blocks = s64.st_blocks.try_into().map_err(|_| io::Errno::OVERFLOW)?;
result.st_atime = s64.st_atime.try_into().map_err(|_| io::Errno::OVERFLOW)?;
result.st_atime = i64::from(s64.st_atime) as _;
result.st_atime_nsec = s64
.st_atime_nsec
.try_into()
.map_err(|_| io::Errno::OVERFLOW)?;
result.st_mtime = s64.st_mtime.try_into().map_err(|_| io::Errno::OVERFLOW)?;
result.st_mtime = i64::from(s64.st_mtime) as _;
result.st_mtime_nsec = s64
.st_mtime_nsec
.try_into()
.map_err(|_| io::Errno::OVERFLOW)?;
result.st_ctime = s64.st_ctime.try_into().map_err(|_| io::Errno::OVERFLOW)?;
result.st_ctime = i64::from(s64.st_ctime) as _;
result.st_ctime_nsec = s64
.st_ctime_nsec
.try_into()
Expand Down
3 changes: 3 additions & 0 deletions src/backend/libc/fs/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -986,10 +986,13 @@ pub struct Stat {
pub st_size: i64,
pub st_blksize: u32,
pub st_blocks: u64,
#[deprecated(note = "Use `rustix::fs::StatExt::atime` instead.")]
pub st_atime: u64,
pub st_atime_nsec: u32,
#[deprecated(note = "Use `rustix::fs::StatExt::mtime` instead.")]
pub st_mtime: u64,
pub st_mtime_nsec: u32,
#[deprecated(note = "Use `rustix::fs::StatExt::ctime` instead.")]
pub st_ctime: u64,
pub st_ctime_nsec: u32,
pub st_ino: u64,
Expand Down
32 changes: 11 additions & 21 deletions src/backend/linux_raw/fs/syscalls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -711,6 +711,7 @@ fn lstat_old(path: &CStr) -> io::Result<Stat> {
target_arch = "mips64",
target_arch = "mips64r6"
))]
#[allow(deprecated)] // for `st_[amc]time` u64->i64 transition
fn statx_to_stat(x: crate::fs::Statx) -> io::Result<Stat> {
Ok(Stat {
st_dev: crate::fs::makedev(x.stx_dev_major, x.stx_dev_minor),
Expand All @@ -722,30 +723,19 @@ fn statx_to_stat(x: crate::fs::Statx) -> io::Result<Stat> {
st_size: x.stx_size.try_into().map_err(|_| io::Errno::OVERFLOW)?,
st_blksize: x.stx_blksize.into(),
st_blocks: x.stx_blocks.into(),
st_atime: x
.stx_atime
.tv_sec
.try_into()
.map_err(|_| io::Errno::OVERFLOW)?,
st_atime: bitcast!(i64::from(x.stx_atime.tv_sec)),
st_atime_nsec: x.stx_atime.tv_nsec.into(),
st_mtime: x
.stx_mtime
.tv_sec
.try_into()
.map_err(|_| io::Errno::OVERFLOW)?,
st_mtime: bitcast!(i64::from(x.stx_mtime.tv_sec)),
st_mtime_nsec: x.stx_mtime.tv_nsec.into(),
st_ctime: x
.stx_ctime
.tv_sec
.try_into()
.map_err(|_| io::Errno::OVERFLOW)?,
st_ctime: bitcast!(i64::from(x.stx_ctime.tv_sec)),
st_ctime_nsec: x.stx_ctime.tv_nsec.into(),
st_ino: x.stx_ino.into(),
})
}

/// Convert from a Linux `stat64` value to rustix's `Stat`.
#[cfg(target_pointer_width = "32")]
#[allow(deprecated)] // for `st_[amc]time` u64->i64 transition
fn stat_to_stat(s64: linux_raw_sys::general::stat64) -> io::Result<Stat> {
Ok(Stat {
st_dev: s64.st_dev.try_into().map_err(|_| io::Errno::OVERFLOW)?,
Expand All @@ -757,17 +747,17 @@ fn stat_to_stat(s64: linux_raw_sys::general::stat64) -> io::Result<Stat> {
st_size: s64.st_size.try_into().map_err(|_| io::Errno::OVERFLOW)?,
st_blksize: s64.st_blksize.try_into().map_err(|_| io::Errno::OVERFLOW)?,
st_blocks: s64.st_blocks.try_into().map_err(|_| io::Errno::OVERFLOW)?,
st_atime: s64.st_atime.try_into().map_err(|_| io::Errno::OVERFLOW)?,
st_atime: bitcast!(i64::from(s64.st_atime)),
st_atime_nsec: s64
.st_atime_nsec
.try_into()
.map_err(|_| io::Errno::OVERFLOW)?,
st_mtime: s64.st_mtime.try_into().map_err(|_| io::Errno::OVERFLOW)?,
st_mtime: bitcast!(i64::from(s64.st_mtime)),
st_mtime_nsec: s64
.st_mtime_nsec
.try_into()
.map_err(|_| io::Errno::OVERFLOW)?,
st_ctime: s64.st_ctime.try_into().map_err(|_| io::Errno::OVERFLOW)?,
st_ctime: bitcast!(i64::from(s64.st_ctime)),
st_ctime_nsec: s64
.st_ctime_nsec
.try_into()
Expand All @@ -789,17 +779,17 @@ fn stat_to_stat(s: linux_raw_sys::general::stat) -> io::Result<Stat> {
st_size: s.st_size.try_into().map_err(|_| io::Errno::OVERFLOW)?,
st_blksize: s.st_blksize.try_into().map_err(|_| io::Errno::OVERFLOW)?,
st_blocks: s.st_blocks.try_into().map_err(|_| io::Errno::OVERFLOW)?,
st_atime: s.st_atime.try_into().map_err(|_| io::Errno::OVERFLOW)?,
st_atime: bitcast!(i64::from(s.st_atime)),
st_atime_nsec: s
.st_atime_nsec
.try_into()
.map_err(|_| io::Errno::OVERFLOW)?,
st_mtime: s.st_mtime.try_into().map_err(|_| io::Errno::OVERFLOW)?,
st_mtime: bitcast!(i64::from(s.st_mtime)),
st_mtime_nsec: s
.st_mtime_nsec
.try_into()
.map_err(|_| io::Errno::OVERFLOW)?,
st_ctime: s.st_ctime.try_into().map_err(|_| io::Errno::OVERFLOW)?,
st_ctime: bitcast!(i64::from(s.st_ctime)),
st_ctime_nsec: s
.st_ctime_nsec
.try_into()
Expand Down
3 changes: 3 additions & 0 deletions src/backend/linux_raw/fs/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -653,10 +653,13 @@ pub struct Stat {
pub st_size: i64,
pub st_blksize: u32,
pub st_blocks: u64,
#[deprecated(note = "Use `rustix::fs::StatExt::atime` instead.")]
pub st_atime: u64,
pub st_atime_nsec: u32,
#[deprecated(note = "Use `rustix::fs::StatExt::mtime` instead.")]
pub st_mtime: u64,
pub st_mtime_nsec: u32,
#[deprecated(note = "Use `rustix::fs::StatExt::ctime` instead.")]
pub st_ctime: u64,
pub st_ctime_nsec: u32,
pub st_ino: u64,
Expand Down
35 changes: 35 additions & 0 deletions src/fs/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,3 +141,38 @@ pub use std::os::unix::fs::{DirEntryExt, FileExt, FileTypeExt, MetadataExt, Open
#[cfg(feature = "std")]
#[cfg(all(wasi_ext, target_os = "wasi"))]
pub use std::os::wasi::fs::{DirEntryExt, FileExt, FileTypeExt, MetadataExt, OpenOptionsExt};

/// Extension trait for accessing timestamp fields of `Stat`.
///
/// Rustix's `Stat` type on some platforms has unsigned `st_mtime`,
/// `st_atime`, and `st_ctime` fields. This is incorrect, as Unix defines
/// these fields to be signed, with negative values representing dates before
/// the Unix epoch. Until the next semver bump, these unsigned fields are
/// deprecated, and this trait provides accessors which return their values
/// as signed integers.
#[cfg(all(unix, not(any(target_os = "aix", target_os = "nto"))))]
pub trait StatExt {
/// Return the value of the `st_atime` field, casted to the correct type.
fn atime(&self) -> i64;
/// Return the value of the `st_mtime` field, casted to the correct type.
fn mtime(&self) -> i64;
/// Return the value of the `st_ctime` field, casted to the correct type.
fn ctime(&self) -> i64;
}

#[cfg(all(unix, not(any(target_os = "aix", target_os = "nto"))))]
#[allow(deprecated)]
impl StatExt for Stat {
#[inline]
fn atime(&self) -> i64 {
self.st_atime as i64
}
#[inline]
fn mtime(&self) -> i64 {
self.st_mtime as i64
}
#[inline]
fn ctime(&self) -> i64 {
self.st_ctime as i64
}
}
4 changes: 2 additions & 2 deletions tests/fs/futimens.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#[cfg(not(any(target_os = "redox", target_os = "wasi")))]
#[test]
fn test_futimens() {
use rustix::fs::{fstat, futimens, openat, Mode, OFlags, Timespec, Timestamps, CWD};
use rustix::fs::{fstat, futimens, openat, Mode, OFlags, StatExt, Timespec, Timestamps, CWD};

let tmp = tempfile::tempdir().unwrap();
let dir = openat(CWD, tmp.path(), OFlags::RDONLY, Mode::empty()).unwrap();
Expand All @@ -28,7 +28,7 @@ fn test_futimens() {

let after = fstat(&file).unwrap();

assert_eq!(times.last_modification.tv_sec as u64, after.st_mtime as u64);
assert_eq!(times.last_modification.tv_sec as u64, after.mtime() as u64);
#[cfg(not(target_os = "netbsd"))]
assert_eq!(
times.last_modification.tv_nsec as u64,
Expand Down
12 changes: 7 additions & 5 deletions tests/fs/utimensat.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
#[cfg(not(any(target_os = "redox", target_os = "wasi")))]
#[test]
fn test_utimensat() {
use rustix::fs::{openat, statat, utimensat, AtFlags, Mode, OFlags, Timespec, Timestamps, CWD};
use rustix::fs::{
openat, statat, utimensat, AtFlags, Mode, OFlags, StatExt, Timespec, Timestamps, CWD,
};

let tmp = tempfile::tempdir().unwrap();
let dir = openat(
Expand Down Expand Up @@ -34,7 +36,7 @@ fn test_utimensat() {

let after = statat(&dir, "file", AtFlags::empty()).unwrap();

assert_eq!(times.last_modification.tv_sec as u64, after.st_mtime as u64);
assert_eq!(times.last_modification.tv_sec as u64, after.mtime() as u64);
#[cfg(not(target_os = "netbsd"))]
assert_eq!(
times.last_modification.tv_nsec as u64,
Expand All @@ -45,15 +47,15 @@ fn test_utimensat() {
times.last_modification.tv_nsec as u64,
after.st_mtimensec as u64
);
assert!(times.last_access.tv_sec as u64 >= after.st_atime as u64);
assert!(times.last_access.tv_sec as u64 >= after.atime() as u64);
#[cfg(not(target_os = "netbsd"))]
assert!(
times.last_access.tv_sec as u64 > after.st_atime as u64
times.last_access.tv_sec as u64 > after.atime() as u64
|| times.last_access.tv_nsec as u64 >= after.st_atime_nsec as u64
);
#[cfg(target_os = "netbsd")]
assert!(
times.last_access.tv_sec as u64 > after.st_atime as u64
times.last_access.tv_sec as u64 > after.atime() as u64
|| times.last_access.tv_nsec as u64 >= after.st_atimensec as u64
);
}
Expand Down
Loading