Skip to content

Commit 6ec74e0

Browse files
sunfishcodesunfishcode
authored
Fix handling of negative timestamps in Stat. (#999)
Deprecate `Stat::st_mtime` etc., because they're unsigned and should be signed, and introduce a `StatExt` extension trait which adds `mtime()` etc. accessor functions which return the values in the appropriate signed type. This `StatExt` trait can go away next time we have a semver breaking change, but for now this preserves compatibility.
1 parent 85eea13 commit 6ec74e0

File tree

8 files changed

+102
-91
lines changed

8 files changed

+102
-91
lines changed

‎src/backend/libc/fs/syscalls.rs‎

Lines changed: 14 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1778,6 +1778,7 @@ pub(crate) fn sendfile(
17781778

17791779
/// Convert from a Linux `statx` value to rustix's `Stat`.
17801780
#[cfg(all(linux_kernel, target_pointer_width = "32"))]
1781+
#[allow(deprecated)] // for `st_[amc]time` u64->i64 transition
17811782
fn statx_to_stat(x: crate::fs::Statx) -> io::Result<Stat> {
17821783
Ok(Stat {
17831784
st_dev: crate::fs::makedev(x.stx_dev_major, x.stx_dev_minor).into(),
@@ -1789,23 +1790,11 @@ fn statx_to_stat(x: crate::fs::Statx) -> io::Result<Stat> {
17891790
st_size: x.stx_size.try_into().map_err(|_| io::Errno::OVERFLOW)?,
17901791
st_blksize: x.stx_blksize.into(),
17911792
st_blocks: x.stx_blocks.into(),
1792-
st_atime: x
1793-
.stx_atime
1794-
.tv_sec
1795-
.try_into()
1796-
.map_err(|_| io::Errno::OVERFLOW)?,
1793+
st_atime: bitcast!(i64::from(x.stx_atime.tv_sec)),
17971794
st_atime_nsec: x.stx_atime.tv_nsec as _,
1798-
st_mtime: x
1799-
.stx_mtime
1800-
.tv_sec
1801-
.try_into()
1802-
.map_err(|_| io::Errno::OVERFLOW)?,
1795+
st_mtime: bitcast!(i64::from(x.stx_mtime.tv_sec)),
18031796
st_mtime_nsec: x.stx_mtime.tv_nsec as _,
1804-
st_ctime: x
1805-
.stx_ctime
1806-
.tv_sec
1807-
.try_into()
1808-
.map_err(|_| io::Errno::OVERFLOW)?,
1797+
st_ctime: bitcast!(i64::from(x.stx_ctime.tv_sec)),
18091798
st_ctime_nsec: x.stx_ctime.tv_nsec as _,
18101799
st_ino: x.stx_ino.into(),
18111800
})
@@ -1827,23 +1816,11 @@ fn statx_to_stat(x: crate::fs::Statx) -> io::Result<Stat> {
18271816
result.st_size = x.stx_size.try_into().map_err(|_| io::Errno::OVERFLOW)?;
18281817
result.st_blksize = x.stx_blksize.into();
18291818
result.st_blocks = x.stx_blocks.try_into().map_err(|_e| io::Errno::OVERFLOW)?;
1830-
result.st_atime = x
1831-
.stx_atime
1832-
.tv_sec
1833-
.try_into()
1834-
.map_err(|_| io::Errno::OVERFLOW)?;
1819+
result.st_atime = bitcast!(i64::from(x.stx_atime.tv_sec));
18351820
result.st_atime_nsec = x.stx_atime.tv_nsec as _;
1836-
result.st_mtime = x
1837-
.stx_mtime
1838-
.tv_sec
1839-
.try_into()
1840-
.map_err(|_| io::Errno::OVERFLOW)?;
1821+
result.st_mtime = bitcast!(i64::from(x.stx_mtime.tv_sec));
18411822
result.st_mtime_nsec = x.stx_mtime.tv_nsec as _;
1842-
result.st_ctime = x
1843-
.stx_ctime
1844-
.tv_sec
1845-
.try_into()
1846-
.map_err(|_| io::Errno::OVERFLOW)?;
1823+
result.st_ctime = bitcast!(i64::from(x.stx_ctime.tv_sec));
18471824
result.st_ctime_nsec = x.stx_ctime.tv_nsec as _;
18481825
result.st_ino = x.stx_ino.into();
18491826

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

18531830
/// Convert from a Linux `stat64` value to rustix's `Stat`.
18541831
#[cfg(all(linux_kernel, target_pointer_width = "32"))]
1832+
#[allow(deprecated)] // for `st_[amc]time` u64->i64 transition
18551833
fn stat64_to_stat(s64: c::stat64) -> io::Result<Stat> {
18561834
Ok(Stat {
18571835
st_dev: s64.st_dev.try_into().map_err(|_| io::Errno::OVERFLOW)?,
@@ -1863,17 +1841,17 @@ fn stat64_to_stat(s64: c::stat64) -> io::Result<Stat> {
18631841
st_size: s64.st_size.try_into().map_err(|_| io::Errno::OVERFLOW)?,
18641842
st_blksize: s64.st_blksize.try_into().map_err(|_| io::Errno::OVERFLOW)?,
18651843
st_blocks: s64.st_blocks.try_into().map_err(|_| io::Errno::OVERFLOW)?,
1866-
st_atime: s64.st_atime.try_into().map_err(|_| io::Errno::OVERFLOW)?,
1844+
st_atime: bitcast!(i64::from(s64.st_atime)),
18671845
st_atime_nsec: s64
18681846
.st_atime_nsec
18691847
.try_into()
18701848
.map_err(|_| io::Errno::OVERFLOW)?,
1871-
st_mtime: s64.st_mtime.try_into().map_err(|_| io::Errno::OVERFLOW)?,
1849+
st_mtime: bitcast!(i64::from(s64.st_mtime)),
18721850
st_mtime_nsec: s64
18731851
.st_mtime_nsec
18741852
.try_into()
18751853
.map_err(|_| io::Errno::OVERFLOW)?,
1876-
st_ctime: s64.st_ctime.try_into().map_err(|_| io::Errno::OVERFLOW)?,
1854+
st_ctime: bitcast!(i64::from(s64.st_ctime)),
18771855
st_ctime_nsec: s64
18781856
.st_ctime_nsec
18791857
.try_into()
@@ -1899,17 +1877,17 @@ fn stat64_to_stat(s64: c::stat64) -> io::Result<Stat> {
18991877
result.st_size = s64.st_size.try_into().map_err(|_| io::Errno::OVERFLOW)?;
19001878
result.st_blksize = s64.st_blksize.try_into().map_err(|_| io::Errno::OVERFLOW)?;
19011879
result.st_blocks = s64.st_blocks.try_into().map_err(|_| io::Errno::OVERFLOW)?;
1902-
result.st_atime = s64.st_atime.try_into().map_err(|_| io::Errno::OVERFLOW)?;
1880+
result.st_atime = i64::from(s64.st_atime) as _;
19031881
result.st_atime_nsec = s64
19041882
.st_atime_nsec
19051883
.try_into()
19061884
.map_err(|_| io::Errno::OVERFLOW)?;
1907-
result.st_mtime = s64.st_mtime.try_into().map_err(|_| io::Errno::OVERFLOW)?;
1885+
result.st_mtime = i64::from(s64.st_mtime) as _;
19081886
result.st_mtime_nsec = s64
19091887
.st_mtime_nsec
19101888
.try_into()
19111889
.map_err(|_| io::Errno::OVERFLOW)?;
1912-
result.st_ctime = s64.st_ctime.try_into().map_err(|_| io::Errno::OVERFLOW)?;
1890+
result.st_ctime = i64::from(s64.st_ctime) as _;
19131891
result.st_ctime_nsec = s64
19141892
.st_ctime_nsec
19151893
.try_into()

‎src/backend/libc/fs/types.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -986,10 +986,13 @@ pub struct Stat {
986986
pub st_size: i64,
987987
pub st_blksize: u32,
988988
pub st_blocks: u64,
989+
#[deprecated(note = "Use `rustix::fs::StatExt::atime` instead.")]
989990
pub st_atime: u64,
990991
pub st_atime_nsec: u32,
992+
#[deprecated(note = "Use `rustix::fs::StatExt::mtime` instead.")]
991993
pub st_mtime: u64,
992994
pub st_mtime_nsec: u32,
995+
#[deprecated(note = "Use `rustix::fs::StatExt::ctime` instead.")]
993996
pub st_ctime: u64,
994997
pub st_ctime_nsec: u32,
995998
pub st_ino: u64,

‎src/backend/linux_raw/fs/syscalls.rs‎

Lines changed: 11 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -711,6 +711,7 @@ fn lstat_old(path: &CStr) -> io::Result<Stat> {
711711
target_arch = "mips64",
712712
target_arch = "mips64r6"
713713
))]
714+
#[allow(deprecated)] // for `st_[amc]time` u64->i64 transition
714715
fn statx_to_stat(x: crate::fs::Statx) -> io::Result<Stat> {
715716
Ok(Stat {
716717
st_dev: crate::fs::makedev(x.stx_dev_major, x.stx_dev_minor),
@@ -722,30 +723,19 @@ fn statx_to_stat(x: crate::fs::Statx) -> io::Result<Stat> {
722723
st_size: x.stx_size.try_into().map_err(|_| io::Errno::OVERFLOW)?,
723724
st_blksize: x.stx_blksize.into(),
724725
st_blocks: x.stx_blocks.into(),
725-
st_atime: x
726-
.stx_atime
727-
.tv_sec
728-
.try_into()
729-
.map_err(|_| io::Errno::OVERFLOW)?,
726+
st_atime: bitcast!(i64::from(x.stx_atime.tv_sec)),
730727
st_atime_nsec: x.stx_atime.tv_nsec.into(),
731-
st_mtime: x
732-
.stx_mtime
733-
.tv_sec
734-
.try_into()
735-
.map_err(|_| io::Errno::OVERFLOW)?,
728+
st_mtime: bitcast!(i64::from(x.stx_mtime.tv_sec)),
736729
st_mtime_nsec: x.stx_mtime.tv_nsec.into(),
737-
st_ctime: x
738-
.stx_ctime
739-
.tv_sec
740-
.try_into()
741-
.map_err(|_| io::Errno::OVERFLOW)?,
730+
st_ctime: bitcast!(i64::from(x.stx_ctime.tv_sec)),
742731
st_ctime_nsec: x.stx_ctime.tv_nsec.into(),
743732
st_ino: x.stx_ino.into(),
744733
})
745734
}
746735

747736
/// Convert from a Linux `stat64` value to rustix's `Stat`.
748737
#[cfg(target_pointer_width = "32")]
738+
#[allow(deprecated)] // for `st_[amc]time` u64->i64 transition
749739
fn stat_to_stat(s64: linux_raw_sys::general::stat64) -> io::Result<Stat> {
750740
Ok(Stat {
751741
st_dev: s64.st_dev.try_into().map_err(|_| io::Errno::OVERFLOW)?,
@@ -757,17 +747,17 @@ fn stat_to_stat(s64: linux_raw_sys::general::stat64) -> io::Result<Stat> {
757747
st_size: s64.st_size.try_into().map_err(|_| io::Errno::OVERFLOW)?,
758748
st_blksize: s64.st_blksize.try_into().map_err(|_| io::Errno::OVERFLOW)?,
759749
st_blocks: s64.st_blocks.try_into().map_err(|_| io::Errno::OVERFLOW)?,
760-
st_atime: s64.st_atime.try_into().map_err(|_| io::Errno::OVERFLOW)?,
750+
st_atime: bitcast!(i64::from(s64.st_atime)),
761751
st_atime_nsec: s64
762752
.st_atime_nsec
763753
.try_into()
764754
.map_err(|_| io::Errno::OVERFLOW)?,
765-
st_mtime: s64.st_mtime.try_into().map_err(|_| io::Errno::OVERFLOW)?,
755+
st_mtime: bitcast!(i64::from(s64.st_mtime)),
766756
st_mtime_nsec: s64
767757
.st_mtime_nsec
768758
.try_into()
769759
.map_err(|_| io::Errno::OVERFLOW)?,
770-
st_ctime: s64.st_ctime.try_into().map_err(|_| io::Errno::OVERFLOW)?,
760+
st_ctime: bitcast!(i64::from(s64.st_ctime)),
771761
st_ctime_nsec: s64
772762
.st_ctime_nsec
773763
.try_into()
@@ -789,17 +779,17 @@ fn stat_to_stat(s: linux_raw_sys::general::stat) -> io::Result<Stat> {
789779
st_size: s.st_size.try_into().map_err(|_| io::Errno::OVERFLOW)?,
790780
st_blksize: s.st_blksize.try_into().map_err(|_| io::Errno::OVERFLOW)?,
791781
st_blocks: s.st_blocks.try_into().map_err(|_| io::Errno::OVERFLOW)?,
792-
st_atime: s.st_atime.try_into().map_err(|_| io::Errno::OVERFLOW)?,
782+
st_atime: bitcast!(i64::from(s.st_atime)),
793783
st_atime_nsec: s
794784
.st_atime_nsec
795785
.try_into()
796786
.map_err(|_| io::Errno::OVERFLOW)?,
797-
st_mtime: s.st_mtime.try_into().map_err(|_| io::Errno::OVERFLOW)?,
787+
st_mtime: bitcast!(i64::from(s.st_mtime)),
798788
st_mtime_nsec: s
799789
.st_mtime_nsec
800790
.try_into()
801791
.map_err(|_| io::Errno::OVERFLOW)?,
802-
st_ctime: s.st_ctime.try_into().map_err(|_| io::Errno::OVERFLOW)?,
792+
st_ctime: bitcast!(i64::from(s.st_ctime)),
803793
st_ctime_nsec: s
804794
.st_ctime_nsec
805795
.try_into()

‎src/backend/linux_raw/fs/types.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -653,10 +653,13 @@ pub struct Stat {
653653
pub st_size: i64,
654654
pub st_blksize: u32,
655655
pub st_blocks: u64,
656+
#[deprecated(note = "Use `rustix::fs::StatExt::atime` instead.")]
656657
pub st_atime: u64,
657658
pub st_atime_nsec: u32,
659+
#[deprecated(note = "Use `rustix::fs::StatExt::mtime` instead.")]
658660
pub st_mtime: u64,
659661
pub st_mtime_nsec: u32,
662+
#[deprecated(note = "Use `rustix::fs::StatExt::ctime` instead.")]
660663
pub st_ctime: u64,
661664
pub st_ctime_nsec: u32,
662665
pub st_ino: u64,

‎src/fs/mod.rs‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,3 +141,38 @@ pub use std::os::unix::fs::{DirEntryExt, FileExt, FileTypeExt, MetadataExt, Open
141141
#[cfg(feature = "std")]
142142
#[cfg(all(wasi_ext, target_os = "wasi"))]
143143
pub use std::os::wasi::fs::{DirEntryExt, FileExt, FileTypeExt, MetadataExt, OpenOptionsExt};
144+
145+
/// Extension trait for accessing timestamp fields of `Stat`.
146+
///
147+
/// Rustix's `Stat` type on some platforms has unsigned `st_mtime`,
148+
/// `st_atime`, and `st_ctime` fields. This is incorrect, as Unix defines
149+
/// these fields to be signed, with negative values representing dates before
150+
/// the Unix epoch. Until the next semver bump, these unsigned fields are
151+
/// deprecated, and this trait provides accessors which return their values
152+
/// as signed integers.
153+
#[cfg(all(unix, not(any(target_os = "aix", target_os = "nto"))))]
154+
pub trait StatExt {
155+
/// Return the value of the `st_atime` field, casted to the correct type.
156+
fn atime(&self) -> i64;
157+
/// Return the value of the `st_mtime` field, casted to the correct type.
158+
fn mtime(&self) -> i64;
159+
/// Return the value of the `st_ctime` field, casted to the correct type.
160+
fn ctime(&self) -> i64;
161+
}
162+
163+
#[cfg(all(unix, not(any(target_os = "aix", target_os = "nto"))))]
164+
#[allow(deprecated)]
165+
impl StatExt for Stat {
166+
#[inline]
167+
fn atime(&self) -> i64 {
168+
self.st_atime as i64
169+
}
170+
#[inline]
171+
fn mtime(&self) -> i64 {
172+
self.st_mtime as i64
173+
}
174+
#[inline]
175+
fn ctime(&self) -> i64 {
176+
self.st_ctime as i64
177+
}
178+
}

‎tests/fs/futimens.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
#[cfg(not(any(target_os = "redox", target_os = "wasi")))]
22
#[test]
33
fn test_futimens() {
4-
use rustix::fs::{fstat, futimens, openat, Mode, OFlags, Timespec, Timestamps, CWD};
4+
use rustix::fs::{fstat, futimens, openat, Mode, OFlags, StatExt, Timespec, Timestamps, CWD};
55

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

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

31-
assert_eq!(times.last_modification.tv_sec as u64, after.st_mtime as u64);
31+
assert_eq!(times.last_modification.tv_sec as u64, after.mtime() as u64);
3232
#[cfg(not(target_os = "netbsd"))]
3333
assert_eq!(
3434
times.last_modification.tv_nsec as u64,

‎tests/fs/utimensat.rs‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
#[cfg(not(any(target_os = "redox", target_os = "wasi")))]
22
#[test]
33
fn test_utimensat() {
4-
use rustix::fs::{openat, statat, utimensat, AtFlags, Mode, OFlags, Timespec, Timestamps, CWD};
4+
use rustix::fs::{
5+
openat, statat, utimensat, AtFlags, Mode, OFlags, StatExt, Timespec, Timestamps, CWD,
6+
};
57

68
let tmp = tempfile::tempdir().unwrap();
79
let dir = openat(
@@ -34,7 +36,7 @@ fn test_utimensat() {
3436

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

37-
assert_eq!(times.last_modification.tv_sec as u64, after.st_mtime as u64);
39+
assert_eq!(times.last_modification.tv_sec as u64, after.mtime() as u64);
3840
#[cfg(not(target_os = "netbsd"))]
3941
assert_eq!(
4042
times.last_modification.tv_nsec as u64,
@@ -45,15 +47,15 @@ fn test_utimensat() {
4547
times.last_modification.tv_nsec as u64,
4648
after.st_mtimensec as u64
4749
);
48-
assert!(times.last_access.tv_sec as u64 >= after.st_atime as u64);
50+
assert!(times.last_access.tv_sec as u64 >= after.atime() as u64);
4951
#[cfg(not(target_os = "netbsd"))]
5052
assert!(
51-
times.last_access.tv_sec as u64 > after.st_atime as u64
53+
times.last_access.tv_sec as u64 > after.atime() as u64
5254
|| times.last_access.tv_nsec as u64 >= after.st_atime_nsec as u64
5355
);
5456
#[cfg(target_os = "netbsd")]
5557
assert!(
56-
times.last_access.tv_sec as u64 > after.st_atime as u64
58+
times.last_access.tv_sec as u64 > after.atime() as u64
5759
|| times.last_access.tv_nsec as u64 >= after.st_atimensec as u64
5860
);
5961
}

0 commit comments

Comments
 (0)