Skip to content

Commit

Permalink
Fix statx created times when unavailable (#268)
Browse files Browse the repository at this point in the history
* Fix statx created times when unavailable

This previously returned `Some(UNIX_EPOCH)` instead of `None` when `statx` did
not return a created time.

* Loosen fs_additional test assertion for created times

This makes it OK to return file created times even if std doesn't,
which is currently the case for musl builds.

* Check `statx` mask to see if `mtime` and `atime` were returned, too

`std` does these checks, so cap-std should too. `std` only does the checks for
32-bit targets, but they don't seem harmful to enable for all targets.
  • Loading branch information
ongardie authored Jul 19, 2022
1 parent 21382e5 commit de73972
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 16 deletions.
20 changes: 16 additions & 4 deletions cap-primitives/src/rustix/fs/metadata_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use crate::fs::{FileTypeExt, Metadata, PermissionsExt};
use crate::time::{Duration, SystemClock, SystemTime};
#[cfg(any(target_os = "android", target_os = "linux"))]
use rustix::fs::{makedev, Statx};
use rustix::fs::{makedev, Statx, StatxFlags};
use rustix::fs::{RawMode, Stat};
use std::convert::{TryFrom, TryInto};
use std::{fs, io};
Expand Down Expand Up @@ -226,9 +226,21 @@ impl MetadataExt {
file_type: FileTypeExt::from_raw_mode(RawMode::from(statx.stx_mode)),
len: u64::try_from(statx.stx_size).unwrap(),
permissions: PermissionsExt::from_raw_mode(RawMode::from(statx.stx_mode)),
modified: system_time_from_rustix(statx.stx_mtime.tv_sec, statx.stx_mtime.tv_nsec as _),
accessed: system_time_from_rustix(statx.stx_atime.tv_sec, statx.stx_atime.tv_nsec as _),
created: system_time_from_rustix(statx.stx_btime.tv_sec, statx.stx_btime.tv_nsec as _),
modified: if statx.stx_mask & StatxFlags::MTIME.bits() != 0 {
system_time_from_rustix(statx.stx_mtime.tv_sec, statx.stx_mtime.tv_nsec as _)
} else {
None
},
accessed: if statx.stx_mask & StatxFlags::ATIME.bits() != 0 {
system_time_from_rustix(statx.stx_atime.tv_sec, statx.stx_atime.tv_nsec as _)
} else {
None
},
created: if statx.stx_mask & StatxFlags::BTIME.bits() != 0 {
system_time_from_rustix(statx.stx_btime.tv_sec, statx.stx_btime.tv_nsec as _)
} else {
None
},

ext: Self {
dev: makedev(statx.stx_dev_major, statx.stx_dev_minor),
Expand Down
52 changes: 40 additions & 12 deletions tests/fs_additional.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
mod sys_common;

use cap_std::fs::{DirBuilder, OpenOptions};
use cap_std::time::SystemClock;
use std::io::{self, Read, Write};
use std::path::Path;
use std::str;
Expand Down Expand Up @@ -936,24 +937,51 @@ fn check_metadata(std: &std::fs::Metadata, cap: &cap_std::fs::Metadata) {

// If the standard library supports file modified/accessed/created times,
// then cap-std should too.
if let Ok(expected) = std.modified() {
assert_eq!(expected, check!(cap.modified()).into_std());
match std.modified() {
Ok(expected) => assert_eq!(expected, check!(cap.modified()).into_std()),
Err(e) => assert!(
cap.modified().is_err(),
"modified time should be error ({}), got {:#?}",
e,
cap.modified()
),
}
// The access times might be a little different due to either our own
// or concurrent accesses.
const ACCESS_TOLERANCE_SEC: u32 = 60;
if let Ok(expected) = std.accessed() {
let access_tolerance = std::time::Duration::from_secs(ACCESS_TOLERANCE_SEC.into());
assert!(
((expected - access_tolerance)..(expected + access_tolerance))
.contains(&check!(cap.accessed()).into_std()),
"std accessed {:#?}, cap accessed {:#?}",
expected,
match std.accessed() {
Ok(expected) => {
let access_tolerance = std::time::Duration::from_secs(ACCESS_TOLERANCE_SEC.into());
assert!(
((expected - access_tolerance)..(expected + access_tolerance))
.contains(&check!(cap.accessed()).into_std()),
"std accessed {:#?}, cap accessed {:#?}",
expected,
cap.accessed()
);
}
Err(e) => assert!(
cap.accessed().is_err(),
"accessed time should be error ({}), got {:#?}",
e,
cap.accessed()
);
),
}
if let Ok(expected) = std.created() {
assert_eq!(expected, check!(cap.created()).into_std());
match std.created() {
Ok(expected) => assert_eq!(expected, check!(cap.created()).into_std()),
Err(e) => {
// An earlier bug returned the Unix epoch instead of `None` when
// created times were unavailable. This tries to catch such errors,
// while also allowing some targets to return valid created times
// even when std doesn't.
if let Ok(actual) = cap.created() {
println!(
"std returned error for created time ({}) but got {:#?}",
e, actual
);
assert_ne!(actual, SystemClock::UNIX_EPOCH);
}
}
}

#[cfg(unix)]
Expand Down

0 comments on commit de73972

Please sign in to comment.