From bc830b91587842bb19ef7254bd1481bafff2dff5 Mon Sep 17 00:00:00 2001 From: Thomas Hurst Date: Mon, 18 May 2020 19:52:59 +0000 Subject: [PATCH 1/5] Handle timestamps before UNIX_EPOCH (#658) Instead of returning a Duration since the epoch from file metadata, which cannot represent times before it, return the SystemTime directly. Move conversion closer to where it's needed, and perform it infallibly. --- src/fs/file.rs | 33 ++++++++-------------- src/output/render/times.rs | 2 +- src/output/time.rs | 58 +++++++++++++++++++++++--------------- 3 files changed, 48 insertions(+), 45 deletions(-) diff --git a/src/fs/file.rs b/src/fs/file.rs index 0d42d54..ee33de1 100644 --- a/src/fs/file.rs +++ b/src/fs/file.rs @@ -4,7 +4,7 @@ use std::io::Error as IOError; use std::io::Result as IOResult; use std::os::unix::fs::{MetadataExt, PermissionsExt, FileTypeExt}; use std::path::{Path, PathBuf}; -use std::time::{UNIX_EPOCH, Duration}; +use std::time::{SystemTime, UNIX_EPOCH}; use log::{debug, error}; @@ -326,35 +326,26 @@ impl<'dir> File<'dir> { } /// This file’s last modified timestamp. - /// If the file's time is invalid, assume it was modified today - pub fn modified_time(&self) -> Duration { - match self.metadata.modified() { - Ok(system_time) => system_time.duration_since(UNIX_EPOCH).unwrap(), - Err(_) => Duration::new(0, 0), - } + /// If the file's time is invalid, assume it was modified at the epoch + pub fn modified_time(&self) -> SystemTime { + self.metadata.modified().unwrap_or(UNIX_EPOCH) } /// This file’s last changed timestamp. - pub fn changed_time(&self) -> Duration { - Duration::new(self.metadata.ctime() as u64, self.metadata.ctime_nsec() as u32) + pub fn changed_time(&self) -> SystemTime { + self.metadata.modified().unwrap_or(UNIX_EPOCH) } /// This file’s last accessed timestamp. - /// If the file's time is invalid, assume it was accessed today - pub fn accessed_time(&self) -> Duration { - match self.metadata.accessed() { - Ok(system_time) => system_time.duration_since(UNIX_EPOCH).unwrap(), - Err(_) => Duration::new(0, 0), - } + /// If the file's time is invalid, assume it was accessed at the epoch + pub fn accessed_time(&self) -> SystemTime { + self.metadata.accessed().unwrap_or(UNIX_EPOCH) } /// This file’s created timestamp. - /// If the file's time is invalid, assume it was created today - pub fn created_time(&self) -> Duration { - match self.metadata.created() { - Ok(system_time) => system_time.duration_since(UNIX_EPOCH).unwrap(), - Err(_) => Duration::new(0, 0), - } + /// If the file's time is invalid, assume it was created at the epoch + pub fn created_time(&self) -> SystemTime { + self.metadata.created().unwrap_or(UNIX_EPOCH) } /// This file’s ‘type’. diff --git a/src/output/render/times.rs b/src/output/render/times.rs index c5c6b3a..2b3d000 100644 --- a/src/output/render/times.rs +++ b/src/output/render/times.rs @@ -11,7 +11,7 @@ pub trait Render { format: &TimeFormat) -> TextCell; } -impl Render for std::time::Duration { +impl Render for std::time::SystemTime { fn render(self, style: Style, tz: &Option, format: &TimeFormat) -> TextCell { diff --git a/src/output/time.rs b/src/output/time.rs index 49b6930..288a729 100644 --- a/src/output/time.rs +++ b/src/output/time.rs @@ -1,6 +1,6 @@ //! Timestamp formatting. -use std::time::Duration; +use std::time::{SystemTime, UNIX_EPOCH}; use datetime::{LocalDateTime, TimeZone, DatePiece, TimePiece}; use datetime::fmt::DateFormat; @@ -51,7 +51,7 @@ pub enum TimeFormat { // timestamps are separate types. impl TimeFormat { - pub fn format_local(&self, time: Duration) -> String { + pub fn format_local(&self, time: SystemTime) -> String { match *self { TimeFormat::DefaultFormat(ref fmt) => fmt.format_local(time), TimeFormat::ISOFormat(ref iso) => iso.format_local(time), @@ -60,7 +60,7 @@ impl TimeFormat { } } - pub fn format_zoned(&self, time: Duration, zone: &TimeZone) -> String { + pub fn format_zoned(&self, time: SystemTime, zone: &TimeZone) -> String { match *self { TimeFormat::DefaultFormat(ref fmt) => fmt.format_zoned(time, zone), TimeFormat::ISOFormat(ref iso) => iso.format_zoned(time, zone), @@ -146,11 +146,11 @@ impl DefaultFormat { } #[allow(trivial_numeric_casts)] - fn format_local(&self, time: Duration) -> String { - if time.as_nanos() == 0 { + fn format_local(&self, time: SystemTime) -> String { + if time == UNIX_EPOCH { return "-".to_string(); } - let date = LocalDateTime::at(time.as_secs() as i64); + let date = LocalDateTime::at(systemtime_epoch(time)); if self.is_recent(date) { format!("{:2} {} {:02}:{:02}", @@ -163,12 +163,12 @@ impl DefaultFormat { } #[allow(trivial_numeric_casts)] - fn format_zoned(&self, time: Duration, zone: &TimeZone) -> String { - if time.as_nanos() == 0 { + fn format_zoned(&self, time: SystemTime, zone: &TimeZone) -> String { + if time == UNIX_EPOCH { return "-".to_string(); } - let date = zone.to_zoned(LocalDateTime::at(time.as_secs() as i64)); + let date = zone.to_zoned(LocalDateTime::at(systemtime_epoch(time))); if self.is_recent(date) { format!("{:2} {} {:02}:{:02}", @@ -181,19 +181,31 @@ impl DefaultFormat { } } +fn systemtime_epoch(time: SystemTime) -> i64 { + time + .duration_since(UNIX_EPOCH) + .map(|t| t.as_secs() as i64) + .unwrap_or_else(|e| -(e.duration().as_secs() as i64)) +} +fn systemtime_nanos(time: SystemTime) -> u32 { + time + .duration_since(UNIX_EPOCH) + .map(|t| t.subsec_nanos()) + .unwrap_or_else(|e| e.duration().subsec_nanos()) +} #[allow(trivial_numeric_casts)] -fn long_local(time: Duration) -> String { - let date = LocalDateTime::at(time.as_secs() as i64); +fn long_local(time: SystemTime) -> String { + let date = LocalDateTime::at(systemtime_epoch(time)); format!("{:04}-{:02}-{:02} {:02}:{:02}", date.year(), date.month() as usize, date.day(), date.hour(), date.minute()) } #[allow(trivial_numeric_casts)] -fn long_zoned(time: Duration, zone: &TimeZone) -> String { - let date = zone.to_zoned(LocalDateTime::at(time.as_secs() as i64)); +fn long_zoned(time: SystemTime, zone: &TimeZone) -> String { + let date = zone.to_zoned(LocalDateTime::at(systemtime_epoch(time))); format!("{:04}-{:02}-{:02} {:02}:{:02}", date.year(), date.month() as usize, date.day(), date.hour(), date.minute()) @@ -201,23 +213,23 @@ fn long_zoned(time: Duration, zone: &TimeZone) -> String { #[allow(trivial_numeric_casts)] -fn full_local(time: Duration) -> String { - let date = LocalDateTime::at(time.as_secs() as i64); +fn full_local(time: SystemTime) -> String { + let date = LocalDateTime::at(systemtime_epoch(time)); format!("{:04}-{:02}-{:02} {:02}:{:02}:{:02}.{:09}", date.year(), date.month() as usize, date.day(), - date.hour(), date.minute(), date.second(), time.subsec_nanos()) + date.hour(), date.minute(), date.second(), systemtime_nanos(time)) } #[allow(trivial_numeric_casts)] -fn full_zoned(time: Duration, zone: &TimeZone) -> String { +fn full_zoned(time: SystemTime, zone: &TimeZone) -> String { use datetime::Offset; - let local = LocalDateTime::at(time.as_secs() as i64); + let local = LocalDateTime::at(systemtime_epoch(time)); let date = zone.to_zoned(local); let offset = Offset::of_seconds(zone.offset(local) as i32).expect("Offset out of range"); format!("{:04}-{:02}-{:02} {:02}:{:02}:{:02}.{:09} {:+03}{:02}", date.year(), date.month() as usize, date.day(), - date.hour(), date.minute(), date.second(), time.subsec_nanos(), + date.hour(), date.minute(), date.second(), systemtime_nanos(time), offset.hours(), offset.minutes().abs()) } @@ -244,8 +256,8 @@ impl ISOFormat { } #[allow(trivial_numeric_casts)] - fn format_local(&self, time: Duration) -> String { - let date = LocalDateTime::at(time.as_secs() as i64); + fn format_local(&self, time: SystemTime) -> String { + let date = LocalDateTime::at(systemtime_epoch(time)); if self.is_recent(date) { format!("{:02}-{:02} {:02}:{:02}", @@ -259,8 +271,8 @@ impl ISOFormat { } #[allow(trivial_numeric_casts)] - fn format_zoned(&self, time: Duration, zone: &TimeZone) -> String { - let date = zone.to_zoned(LocalDateTime::at(time.as_secs() as i64)); + fn format_zoned(&self, time: SystemTime, zone: &TimeZone) -> String { + let date = zone.to_zoned(LocalDateTime::at(systemtime_epoch(time))); if self.is_recent(date) { format!("{:02}-{:02} {:02}:{:02}", From d2d2e7325f1fe03d3407a71cd79d9ffa52c8f892 Mon Sep 17 00:00:00 2001 From: Thomas Hurst Date: Mon, 18 May 2020 21:25:07 +0000 Subject: [PATCH 2/5] Correct handling of pre-epoch timestamps Fix an off-by-one on the seconds when subseconds are present, and correct display of nenoseconds, which are of course inverted due to the internal value being negative. --- src/output/time.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/output/time.rs b/src/output/time.rs index 288a729..1d0138d 100644 --- a/src/output/time.rs +++ b/src/output/time.rs @@ -185,14 +185,23 @@ fn systemtime_epoch(time: SystemTime) -> i64 { time .duration_since(UNIX_EPOCH) .map(|t| t.as_secs() as i64) - .unwrap_or_else(|e| -(e.duration().as_secs() as i64)) + .unwrap_or_else(|e| { + -(e.duration().as_secs() as i64) - e.duration().subsec_nanos().min(1) as i64 + }) } fn systemtime_nanos(time: SystemTime) -> u32 { time .duration_since(UNIX_EPOCH) .map(|t| t.subsec_nanos()) - .unwrap_or_else(|e| e.duration().subsec_nanos()) + .unwrap_or_else(|e| { + let nanos = e.duration().subsec_nanos(); + if nanos == 0 { + 0 + } else { + 1_000_000_000 - nanos + } + }) } #[allow(trivial_numeric_casts)] From 86163ab29890c427c23fc79b26e0fca65c28b933 Mon Sep 17 00:00:00 2001 From: Thomas Hurst Date: Mon, 18 May 2020 23:06:27 +0000 Subject: [PATCH 3/5] Restore ctime handling with correct pre-epoch behaviour --- src/fs/file.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/fs/file.rs b/src/fs/file.rs index ee33de1..35b2940 100644 --- a/src/fs/file.rs +++ b/src/fs/file.rs @@ -4,7 +4,7 @@ use std::io::Error as IOError; use std::io::Result as IOResult; use std::os::unix::fs::{MetadataExt, PermissionsExt, FileTypeExt}; use std::path::{Path, PathBuf}; -use std::time::{SystemTime, UNIX_EPOCH}; +use std::time::{Duration, SystemTime, UNIX_EPOCH}; use log::{debug, error}; @@ -333,7 +333,17 @@ impl<'dir> File<'dir> { /// This file’s last changed timestamp. pub fn changed_time(&self) -> SystemTime { - self.metadata.modified().unwrap_or(UNIX_EPOCH) + let (mut sec, mut nsec) = (self.metadata.ctime(), self.metadata.ctime_nsec()); + + if sec < 0 { // yeah right + if nsec > 0 { + sec += 1; + nsec = nsec - 1_000_000_000; + } + UNIX_EPOCH - Duration::new(sec.abs() as u64, nsec.abs() as u32) + } else { + UNIX_EPOCH + Duration::new(sec as u64, nsec as u32) + } } /// This file’s last accessed timestamp. From e54e1f53c83be3e62dc945d44cb569b79ac68ec7 Mon Sep 17 00:00:00 2001 From: Thomas Hurst Date: Mon, 18 May 2020 23:23:30 +0000 Subject: [PATCH 4/5] Make logic a little clearer --- src/output/time.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/output/time.rs b/src/output/time.rs index 1d0138d..13a6a1d 100644 --- a/src/output/time.rs +++ b/src/output/time.rs @@ -186,7 +186,12 @@ fn systemtime_epoch(time: SystemTime) -> i64 { .duration_since(UNIX_EPOCH) .map(|t| t.as_secs() as i64) .unwrap_or_else(|e| { - -(e.duration().as_secs() as i64) - e.duration().subsec_nanos().min(1) as i64 + let diff = e.duration(); + let mut secs = diff.as_secs(); + if diff.subsec_nanos() > 0 { + secs += 1; + } + -(secs as i64) }) } @@ -196,10 +201,10 @@ fn systemtime_nanos(time: SystemTime) -> u32 { .map(|t| t.subsec_nanos()) .unwrap_or_else(|e| { let nanos = e.duration().subsec_nanos(); - if nanos == 0 { - 0 - } else { + if nanos > 0 { 1_000_000_000 - nanos + } else { + nanos } }) } From acb7c49abf955691469473d92fe9cb2fbfde3edc Mon Sep 17 00:00:00 2001 From: Thomas Hurst Date: Tue, 19 May 2020 02:31:15 +0000 Subject: [PATCH 5/5] Improve handling of unavailable timestamps. Previously if a timestamp was unavailable, it defaulted to the epoch. Prior to this it defaulted to a zero duration. Switch to an Option and move the handling of unavailable timestamps to rendering. --- src/fs/file.rs | 45 +++++++++++++++++++------------------- src/output/render/times.rs | 21 ++++++++++-------- src/output/time.rs | 7 ------ 3 files changed, 34 insertions(+), 39 deletions(-) diff --git a/src/fs/file.rs b/src/fs/file.rs index 35b2940..5745208 100644 --- a/src/fs/file.rs +++ b/src/fs/file.rs @@ -325,37 +325,36 @@ impl<'dir> File<'dir> { } } - /// This file’s last modified timestamp. - /// If the file's time is invalid, assume it was modified at the epoch - pub fn modified_time(&self) -> SystemTime { - self.metadata.modified().unwrap_or(UNIX_EPOCH) + /// This file’s last modified timestamp, if available on this platform. + pub fn modified_time(&self) -> Option { + self.metadata.modified().ok() } - /// This file’s last changed timestamp. - pub fn changed_time(&self) -> SystemTime { + /// This file’s last changed timestamp, if available on this platform. + pub fn changed_time(&self) -> Option { let (mut sec, mut nsec) = (self.metadata.ctime(), self.metadata.ctime_nsec()); - if sec < 0 { // yeah right - if nsec > 0 { - sec += 1; - nsec = nsec - 1_000_000_000; - } - UNIX_EPOCH - Duration::new(sec.abs() as u64, nsec.abs() as u32) - } else { - UNIX_EPOCH + Duration::new(sec as u64, nsec as u32) - } + Some( + if sec < 0 { + if nsec > 0 { + sec += 1; + nsec = nsec - 1_000_000_000; + } + UNIX_EPOCH - Duration::new(sec.abs() as u64, nsec.abs() as u32) + } else { + UNIX_EPOCH + Duration::new(sec as u64, nsec as u32) + } + ) } - /// This file’s last accessed timestamp. - /// If the file's time is invalid, assume it was accessed at the epoch - pub fn accessed_time(&self) -> SystemTime { - self.metadata.accessed().unwrap_or(UNIX_EPOCH) + /// This file’s last accessed timestamp, if available on this platform. + pub fn accessed_time(&self) -> Option { + self.metadata.accessed().ok() } - /// This file’s created timestamp. - /// If the file's time is invalid, assume it was created at the epoch - pub fn created_time(&self) -> SystemTime { - self.metadata.created().unwrap_or(UNIX_EPOCH) + /// This file’s created timestamp, if available on this platform. + pub fn created_time(&self) -> Option { + self.metadata.created().ok() } /// This file’s ‘type’. diff --git a/src/output/render/times.rs b/src/output/render/times.rs index 2b3d000..135be35 100644 --- a/src/output/render/times.rs +++ b/src/output/render/times.rs @@ -11,18 +11,21 @@ pub trait Render { format: &TimeFormat) -> TextCell; } -impl Render for std::time::SystemTime { +impl Render for Option { fn render(self, style: Style, tz: &Option, format: &TimeFormat) -> TextCell { - if let Some(ref tz) = *tz { - let datestamp = format.format_zoned(self, tz); - TextCell::paint(style, datestamp) - } - else { - let datestamp = format.format_local(self); - TextCell::paint(style, datestamp) - } + let datestamp = if let Some(time) = self { + if let Some(ref tz) = tz { + format.format_zoned(time, tz) + } else { + format.format_local(time) + } + } else { + String::from("-") + }; + + TextCell::paint(style, datestamp) } } diff --git a/src/output/time.rs b/src/output/time.rs index 13a6a1d..b168c6b 100644 --- a/src/output/time.rs +++ b/src/output/time.rs @@ -147,9 +147,6 @@ impl DefaultFormat { #[allow(trivial_numeric_casts)] fn format_local(&self, time: SystemTime) -> String { - if time == UNIX_EPOCH { - return "-".to_string(); - } let date = LocalDateTime::at(systemtime_epoch(time)); if self.is_recent(date) { @@ -164,10 +161,6 @@ impl DefaultFormat { #[allow(trivial_numeric_casts)] fn format_zoned(&self, time: SystemTime, zone: &TimeZone) -> String { - if time == UNIX_EPOCH { - return "-".to_string(); - } - let date = zone.to_zoned(LocalDateTime::at(systemtime_epoch(time))); if self.is_recent(date) {