public inbox for pmg-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pmg-devel] [PATCH log-tracker] drop 'time' dependency
@ 2022-01-04 13:57 Wolfgang Bumiller
  2022-01-05 19:01 ` Stoiko Ivanov
  2022-02-14 18:09 ` Stoiko Ivanov
  0 siblings, 2 replies; 3+ messages in thread
From: Wolfgang Bumiller @ 2022-01-04 13:57 UTC (permalink / raw)
  To: pmg-devel

We're using a very old version of it and the functions we
actually use are available in glibc anyway.

The only difference I found was that the result of
glibc's `strptime()`'s `%s` does *not* want an additional
`.to_local()` call anymore...

... which was weird anyway.

As a minor optimization I pass the format string as `&CStr`
to avoid the memcpy.

(The CString::new() call in `strptime()` only happens during
parameter parsing)

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 Cargo.toml  |   1 -
 src/main.rs |  75 ++++++++---------------
 src/time.rs | 169 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 194 insertions(+), 51 deletions(-)
 create mode 100644 src/time.rs

diff --git a/Cargo.toml b/Cargo.toml
index a975d39..7c31157 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -14,4 +14,3 @@ anyhow = "1"
 clap = "2.32"
 flate2 = "1.0"
 libc = "0.2.48"
-time = "0.1.42"
diff --git a/src/main.rs b/src/main.rs
index fb06463..bf9783b 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -15,6 +15,9 @@ use libc::time_t;
 
 use clap::{App, Arg};
 
+mod time;
+use time::{Tm, CAL_MTOD};
+
 fn main() -> Result<(), Error> {
     let matches = App::new(clap::crate_name!())
         .version(clap::crate_version!())
@@ -114,7 +117,7 @@ fn main() -> Result<(), Error> {
         )
         .get_matches();
 
-    let mut parser = Parser::new();
+    let mut parser = Parser::new()?;
     parser.handle_args(matches)?;
 
     println!("# LogReader: {}", std::process::id());
@@ -144,12 +147,12 @@ fn main() -> Result<(), Error> {
 
     println!(
         "# Start: {} ({})",
-        time::strftime("%F %T", &parser.start_tm)?,
+        time::strftime(c_str!("%F %T"), &parser.start_tm)?,
         parser.options.start
     );
     println!(
         "# End: {} ({})",
-        time::strftime("%F %T", &parser.end_tm)?,
+        time::strftime(c_str!("%F %T"), &parser.end_tm)?,
         parser.options.end
     );
 
@@ -1743,10 +1746,10 @@ struct Parser {
 }
 
 impl Parser {
-    fn new() -> Self {
-        let ltime = time::now();
+    fn new() -> Result<Self, Error> {
+        let ltime = Tm::now_local()?;
 
-        Self {
+        Ok(Self {
             sentries: HashMap::new(),
             fentries: HashMap::new(),
             qentries: HashMap::new(),
@@ -1760,12 +1763,12 @@ impl Parser {
             count: 0,
             buffered_stdout: BufWriter::with_capacity(4 * 1024 * 1024, std::io::stdout()),
             options: Options::default(),
-            start_tm: time::empty_tm(),
-            end_tm: time::empty_tm(),
+            start_tm: Tm::zero(),
+            end_tm: Tm::zero(),
             ctime: 0,
             string_match: false,
             lines: 0,
-        }
+        })
     }
 
     fn free_sentry(&mut self, sentry_pid: u64) {
@@ -1976,40 +1979,38 @@ impl Parser {
         }
 
         if let Some(start) = args.value_of("start") {
-            if let Ok(res) = time::strptime(start, "%F %T") {
-                self.options.start = mkgmtime(&res);
+            if let Ok(res) = time::strptime(start, c_str!("%F %T")) {
+                self.options.start = res.as_utc_to_epoch();
                 self.start_tm = res;
-            } else if let Ok(res) = time::strptime(start, "%s") {
-                let res = res.to_local();
-                self.options.start = mkgmtime(&res);
+            } else if let Ok(res) = time::strptime(start, c_str!("%s")) {
+                self.options.start = res.as_utc_to_epoch();
                 self.start_tm = res;
             } else {
                 bail!("failed to parse start time");
             }
         } else {
-            let mut ltime = time::now();
+            let mut ltime = Tm::now_local()?;
             ltime.tm_sec = 0;
             ltime.tm_min = 0;
             ltime.tm_hour = 0;
-            self.options.start = mkgmtime(&ltime);
+            self.options.start = ltime.as_utc_to_epoch();
             self.start_tm = ltime;
         }
 
         if let Some(end) = args.value_of("end") {
-            if let Ok(res) = time::strptime(end, "%F %T") {
-                self.options.end = mkgmtime(&res);
+            if let Ok(res) = time::strptime(end, c_str!("%F %T")) {
+                self.options.end = res.as_utc_to_epoch();
                 self.end_tm = res;
-            } else if let Ok(res) = time::strptime(end, "%s") {
-                let res = res.to_local();
-                self.options.end = mkgmtime(&res);
+            } else if let Ok(res) = time::strptime(end, c_str!("%s")) {
+                self.options.end = res.as_utc_to_epoch();
                 self.end_tm = res;
             } else {
                 bail!("failed to parse end time");
             }
         } else {
-            let ltime = time::now();
-            self.options.end = mkgmtime(&ltime);
-            self.end_tm = ltime;
+            // FIXME: just use libc::time(NULL) here
+            self.options.end = unsafe { libc::time(std::ptr::null_mut()) };
+            self.end_tm = Tm::at_local(self.options.end)?;
         }
 
         if self.options.end < self.options.start {
@@ -2171,30 +2172,6 @@ fn get_or_create_fentry(
     }
 }
 
-fn mkgmtime(tm: &time::Tm) -> time_t {
-    let mut res: time_t;
-
-    let mut year = tm.tm_year as i64 + 1900;
-    let mon = tm.tm_mon;
-
-    res = (year - 1970) * 365 + CAL_MTOD[mon as usize];
-
-    if mon <= 1 {
-        year -= 1;
-    }
-
-    res += (year - 1968) / 4;
-    res -= (year - 1900) / 100;
-    res += (year - 1600) / 400;
-
-    res += (tm.tm_mday - 1) as i64;
-    res = res * 24 + tm.tm_hour as i64;
-    res = res * 60 + tm.tm_min as i64;
-    res = res * 60 + tm.tm_sec as i64;
-
-    res
-}
-
 const LOGFILES: [&str; 32] = [
     "/var/log/syslog",
     "/var/log/syslog.1",
@@ -2397,8 +2374,6 @@ fn parse_time<'a>(
     Some((ltime, data))
 }
 
-const CAL_MTOD: [i64; 12] = [0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334];
-
 type ByteSlice<'a> = &'a [u8];
 /// Parse Host, Service and PID at the beginning of data. Returns a tuple of (host, service, pid, remaining_text).
 fn parse_host_service_pid(data: &[u8]) -> Option<(ByteSlice, ByteSlice, u64, ByteSlice)> {
diff --git a/src/time.rs b/src/time.rs
new file mode 100644
index 0000000..5851496
--- /dev/null
+++ b/src/time.rs
@@ -0,0 +1,169 @@
+//! Time support library.
+//!
+//! Contains wrappers for `strftime`, `strptime`, `libc::localtime_r`.
+
+use std::ffi::{CStr, CString};
+use std::fmt;
+use std::mem::MaybeUninit;
+
+use anyhow::{bail, format_err, Error};
+
+/// Calender month index to *non-leap-year* day-of-the-year.
+pub const CAL_MTOD: [i64; 12] = [0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334];
+
+/// Shortcut for generating an `&'static CStr`.
+#[macro_export]
+macro_rules! c_str {
+    ($data:expr) => {{
+        let bytes = concat!($data, "\0");
+        unsafe { ::std::ffi::CStr::from_bytes_with_nul_unchecked(bytes.as_bytes()) }
+    }};
+}
+
+// Wrapper around `libc::tm` providing `Debug` and some helper methods.
+pub struct Tm(libc::tm);
+
+impl std::ops::Deref for Tm {
+    type Target = libc::tm;
+
+    fn deref(&self) -> &libc::tm {
+        &self.0
+    }
+}
+
+impl std::ops::DerefMut for Tm {
+    fn deref_mut(&mut self) -> &mut libc::tm {
+        &mut self.0
+    }
+}
+
+// (or add libc feature 'extra-traits' but that's the only struct we need it for...)
+impl fmt::Debug for Tm {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        f.debug_struct("Tm")
+            .field("tm_sec", &self.tm_sec)
+            .field("tm_min", &self.tm_min)
+            .field("tm_hour", &self.tm_hour)
+            .field("tm_mday", &self.tm_mday)
+            .field("tm_mon", &self.tm_mon)
+            .field("tm_year", &self.tm_year)
+            .field("tm_wday", &self.tm_wday)
+            .field("tm_yday", &self.tm_yday)
+            .field("tm_isdst", &self.tm_isdst)
+            .field("tm_gmtoff", &self.tm_gmtoff)
+            .field("tm_zone", &self.tm_zone)
+            .finish()
+    }
+}
+
+/// These are now exposed via `libc`, but they're part of glibc.
+mod imports {
+    extern "C" {
+        pub fn strftime(
+            s: *mut libc::c_char,
+            max: libc::size_t,
+            format: *const libc::c_char,
+            tm: *const libc::tm,
+        ) -> libc::size_t;
+
+        pub fn strptime(
+            s: *const libc::c_char,
+            format: *const libc::c_char,
+            tm: *mut libc::tm,
+        ) -> *const libc::c_char;
+    }
+}
+
+impl Tm {
+    /// A zero-initialized time struct.
+    #[inline(always)]
+    pub fn zero() -> Self {
+        unsafe { std::mem::zeroed() }
+    }
+
+    /// Get the current time in the local timezone.
+    pub fn now_local() -> Result<Self, Error> {
+        Self::at_local(unsafe { libc::time(std::ptr::null_mut()) })
+    }
+
+    /// Get a local time from a unix time stamp.
+    pub fn at_local(time: libc::time_t) -> Result<Self, Error> {
+        let mut out = MaybeUninit::<libc::tm>::uninit();
+        Ok(Self(unsafe {
+            if libc::localtime_r(&time, out.as_mut_ptr()).is_null() {
+                bail!("failed to convert timestamp to local time");
+            }
+            out.assume_init()
+        }))
+    }
+
+    /// Assume this is an UTC time and convert it to a unix time stamp.
+    ///
+    /// Equivalent to `timegm(3)`, the gmt equivalent of `mktime(3)`.
+    pub fn as_utc_to_epoch(&self) -> libc::time_t {
+        let mut year = self.0.tm_year as i64 + 1900;
+        let mon = self.0.tm_mon;
+
+        let mut res: libc::time_t = (year - 1970) * 365 + CAL_MTOD[mon as usize];
+
+        if mon <= 1 {
+            year -= 1;
+        }
+
+        res += (year - 1968) / 4;
+        res -= (year - 1900) / 100;
+        res += (year - 1600) / 400;
+
+        res += (self.0.tm_mday - 1) as i64;
+        res = res * 24 + self.0.tm_hour as i64;
+        res = res * 60 + self.0.tm_min as i64;
+        res = res * 60 + self.0.tm_sec as i64;
+
+        res
+    }
+}
+
+/// Wrapper around `strftime(3)` to format time strings.
+pub fn strftime(format: &CStr, tm: &Tm) -> Result<String, Error> {
+    let mut buf = MaybeUninit::<[u8; 64]>::uninit();
+
+    let size = unsafe {
+        imports::strftime(
+            buf.as_mut_ptr() as *mut libc::c_char,
+            64,
+            format.as_ptr(),
+            &tm.0,
+        )
+    };
+    if size == 0 {
+        bail!("failed to format time");
+    }
+    let size = size as usize;
+
+    let buf = unsafe { buf.assume_init() };
+
+    std::str::from_utf8(&buf[..size])
+        .map_err(|_| format_err!("formatted time was not valid utf-8"))
+        .map(str::to_string)
+}
+
+/// Wrapper around `strptime(3)` to parse time strings.
+pub fn strptime(time: &str, format: &CStr) -> Result<Tm, Error> {
+    let mut out = MaybeUninit::<libc::tm>::uninit();
+
+    let time = CString::new(time).map_err(|_| format_err!("time string contains nul bytes"))?;
+
+    let end = unsafe {
+        imports::strptime(
+            time.as_ptr() as *const libc::c_char,
+            format.as_ptr(),
+            out.as_mut_ptr(),
+        )
+    };
+
+    if end.is_null() {
+        bail!("failed to parse time string {:?}", time);
+    }
+
+    Ok(Tm(unsafe { out.assume_init() }))
+}
-- 
2.30.2





^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [pmg-devel] [PATCH log-tracker] drop 'time' dependency
  2022-01-04 13:57 [pmg-devel] [PATCH log-tracker] drop 'time' dependency Wolfgang Bumiller
@ 2022-01-05 19:01 ` Stoiko Ivanov
  2022-02-14 18:09 ` Stoiko Ivanov
  1 sibling, 0 replies; 3+ messages in thread
From: Stoiko Ivanov @ 2022-01-05 19:01 UTC (permalink / raw)
  To: Wolfgang Bumiller; +Cc: pmg-devel

huge thanks for taking the time ;) to clean-up!


On Tue,  4 Jan 2022 14:57:23 +0100
Wolfgang Bumiller <w.bumiller@proxmox.com> wrote:

> We're using a very old version of it and the functions we
> actually use are available in glibc anyway.
> 
> The only difference I found was that the result of
> glibc's `strptime()`'s `%s` does *not* want an additional
> `.to_local()` call anymore...
> 
> ... which was weird anyway.
since I'm always a bit cautious with "weird" in context of time(zone)
handling (which I do consider a bit brittle) - I tried to find out why
this was needed before and not anymore (took me quite a while)

glibc's strptime - calls localtime_r on the parsed number:
https://code.woboq.org/userspace/glibc/time/strptime_l.c.html#678

OTOH the time crate (in version 0.1.42) skips this step (or rather on the
contrary actively calls `at_utc` on the argument (which calls
sys::time_to_utc_tm with an tm struct with an offset of '0')

in any case since sys::time_to_utc_tm and sys::time_to_local_tm
only differ in the comment:
```
// FIXME: Add timezone logic
```
I am quite confident that your version is more correct (or at least
matches glibc's behaviour better

(might also explain why there's `at_utc` and `at` in the crate but not an
explicit `at_local` ;)

Additionally I did a bit of testing (and am really grateful for the tests
we already have in the repository) - and in addition to the code - the
output also looks correct

With that:

Reviewed-By: Stoiko Ivanov <s.ivanov@proxmox.com>
Tested-By: Stoiko Ivanov <s.ivanov@proxmox.com>

(I'm not directly applying it, in the hope that someone more experienced
with the log-tracker could also throw a quick glance at this)

small nit (maybe as hint to the person applying):
> 
> As a minor optimization I pass the format string as `&CStr`
> to avoid the memcpy.
> 
> (The CString::new() call in `strptime()` only happens during
> parameter parsing)
> 
> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> ---
>  Cargo.toml  |   1 -
>  src/main.rs |  75 ++++++++---------------
>  src/time.rs | 169 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 194 insertions(+), 51 deletions(-)
>  create mode 100644 src/time.rs
> 
> diff --git a/Cargo.toml b/Cargo.toml
> index a975d39..7c31157 100644
> --- a/Cargo.toml
> +++ b/Cargo.toml
> @@ -14,4 +14,3 @@ anyhow = "1"
>  clap = "2.32"
>  flate2 = "1.0"
>  libc = "0.2.48"
> -time = "0.1.42"
> diff --git a/src/main.rs b/src/main.rs
> index fb06463..bf9783b 100644
> --- a/src/main.rs
> +++ b/src/main.rs
> @@ -15,6 +15,9 @@ use libc::time_t;
>  
>  use clap::{App, Arg};
>  
> +mod time;
> +use time::{Tm, CAL_MTOD};
> +
>  fn main() -> Result<(), Error> {
>      let matches = App::new(clap::crate_name!())
>          .version(clap::crate_version!())
> @@ -114,7 +117,7 @@ fn main() -> Result<(), Error> {
>          )
>          .get_matches();
>  
> -    let mut parser = Parser::new();
> +    let mut parser = Parser::new()?;
>      parser.handle_args(matches)?;
>  
>      println!("# LogReader: {}", std::process::id());
> @@ -144,12 +147,12 @@ fn main() -> Result<(), Error> {
>  
>      println!(
>          "# Start: {} ({})",
> -        time::strftime("%F %T", &parser.start_tm)?,
> +        time::strftime(c_str!("%F %T"), &parser.start_tm)?,
>          parser.options.start
>      );
>      println!(
>          "# End: {} ({})",
> -        time::strftime("%F %T", &parser.end_tm)?,
> +        time::strftime(c_str!("%F %T"), &parser.end_tm)?,
>          parser.options.end
>      );
>  
> @@ -1743,10 +1746,10 @@ struct Parser {
>  }
>  
>  impl Parser {
> -    fn new() -> Self {
> -        let ltime = time::now();
> +    fn new() -> Result<Self, Error> {
> +        let ltime = Tm::now_local()?;
>  
> -        Self {
> +        Ok(Self {
>              sentries: HashMap::new(),
>              fentries: HashMap::new(),
>              qentries: HashMap::new(),
> @@ -1760,12 +1763,12 @@ impl Parser {
>              count: 0,
>              buffered_stdout: BufWriter::with_capacity(4 * 1024 * 1024, std::io::stdout()),
>              options: Options::default(),
> -            start_tm: time::empty_tm(),
> -            end_tm: time::empty_tm(),
> +            start_tm: Tm::zero(),
> +            end_tm: Tm::zero(),
>              ctime: 0,
>              string_match: false,
>              lines: 0,
> -        }
> +        })
>      }
>  
>      fn free_sentry(&mut self, sentry_pid: u64) {
> @@ -1976,40 +1979,38 @@ impl Parser {
>          }
>  
>          if let Some(start) = args.value_of("start") {
> -            if let Ok(res) = time::strptime(start, "%F %T") {
> -                self.options.start = mkgmtime(&res);
> +            if let Ok(res) = time::strptime(start, c_str!("%F %T")) {
> +                self.options.start = res.as_utc_to_epoch();
>                  self.start_tm = res;
> -            } else if let Ok(res) = time::strptime(start, "%s") {
> -                let res = res.to_local();
> -                self.options.start = mkgmtime(&res);
> +            } else if let Ok(res) = time::strptime(start, c_str!("%s")) {
> +                self.options.start = res.as_utc_to_epoch();
>                  self.start_tm = res;
>              } else {
>                  bail!("failed to parse start time");
>              }
>          } else {
> -            let mut ltime = time::now();
> +            let mut ltime = Tm::now_local()?;
>              ltime.tm_sec = 0;
>              ltime.tm_min = 0;
>              ltime.tm_hour = 0;
> -            self.options.start = mkgmtime(&ltime);
> +            self.options.start = ltime.as_utc_to_epoch();
>              self.start_tm = ltime;
>          }
>  
>          if let Some(end) = args.value_of("end") {
> -            if let Ok(res) = time::strptime(end, "%F %T") {
> -                self.options.end = mkgmtime(&res);
> +            if let Ok(res) = time::strptime(end, c_str!("%F %T")) {
> +                self.options.end = res.as_utc_to_epoch();
>                  self.end_tm = res;
> -            } else if let Ok(res) = time::strptime(end, "%s") {
> -                let res = res.to_local();
> -                self.options.end = mkgmtime(&res);
> +            } else if let Ok(res) = time::strptime(end, c_str!("%s")) {
> +                self.options.end = res.as_utc_to_epoch();
>                  self.end_tm = res;
>              } else {
>                  bail!("failed to parse end time");
>              }
>          } else {
> -            let ltime = time::now();
> -            self.options.end = mkgmtime(&ltime);
> -            self.end_tm = ltime;
> +            // FIXME: just use libc::time(NULL) here
this comment is addressed and can be removed

> +            self.options.end = unsafe { libc::time(std::ptr::null_mut()) };
> +            self.end_tm = Tm::at_local(self.options.end)?;
>          }
>  
>          if self.options.end < self.options.start {
> @@ -2171,30 +2172,6 @@ fn get_or_create_fentry(
>      }
>  }
>  
> -fn mkgmtime(tm: &time::Tm) -> time_t {
> -    let mut res: time_t;
> -
> -    let mut year = tm.tm_year as i64 + 1900;
> -    let mon = tm.tm_mon;
> -
> -    res = (year - 1970) * 365 + CAL_MTOD[mon as usize];
> -
> -    if mon <= 1 {
> -        year -= 1;
> -    }
> -
> -    res += (year - 1968) / 4;
> -    res -= (year - 1900) / 100;
> -    res += (year - 1600) / 400;
> -
> -    res += (tm.tm_mday - 1) as i64;
> -    res = res * 24 + tm.tm_hour as i64;
> -    res = res * 60 + tm.tm_min as i64;
> -    res = res * 60 + tm.tm_sec as i64;
> -
> -    res
> -}
> -
>  const LOGFILES: [&str; 32] = [
>      "/var/log/syslog",
>      "/var/log/syslog.1",
> @@ -2397,8 +2374,6 @@ fn parse_time<'a>(
>      Some((ltime, data))
>  }
>  
> -const CAL_MTOD: [i64; 12] = [0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334];
> -
>  type ByteSlice<'a> = &'a [u8];
>  /// Parse Host, Service and PID at the beginning of data. Returns a tuple of (host, service, pid, remaining_text).
>  fn parse_host_service_pid(data: &[u8]) -> Option<(ByteSlice, ByteSlice, u64, ByteSlice)> {
> diff --git a/src/time.rs b/src/time.rs
> new file mode 100644
> index 0000000..5851496
> --- /dev/null
> +++ b/src/time.rs
> @@ -0,0 +1,169 @@
> +//! Time support library.
> +//!
> +//! Contains wrappers for `strftime`, `strptime`, `libc::localtime_r`.
> +
> +use std::ffi::{CStr, CString};
> +use std::fmt;
> +use std::mem::MaybeUninit;
> +
> +use anyhow::{bail, format_err, Error};
> +
> +/// Calender month index to *non-leap-year* day-of-the-year.
> +pub const CAL_MTOD: [i64; 12] = [0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334];
> +
> +/// Shortcut for generating an `&'static CStr`.
> +#[macro_export]
> +macro_rules! c_str {
> +    ($data:expr) => {{
> +        let bytes = concat!($data, "\0");
> +        unsafe { ::std::ffi::CStr::from_bytes_with_nul_unchecked(bytes.as_bytes()) }
> +    }};
> +}
> +
> +// Wrapper around `libc::tm` providing `Debug` and some helper methods.
> +pub struct Tm(libc::tm);
> +
> +impl std::ops::Deref for Tm {
> +    type Target = libc::tm;
> +
> +    fn deref(&self) -> &libc::tm {
> +        &self.0
> +    }
> +}
> +
> +impl std::ops::DerefMut for Tm {
> +    fn deref_mut(&mut self) -> &mut libc::tm {
> +        &mut self.0
> +    }
> +}
> +
> +// (or add libc feature 'extra-traits' but that's the only struct we need it for...)
> +impl fmt::Debug for Tm {
> +    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
> +        f.debug_struct("Tm")
> +            .field("tm_sec", &self.tm_sec)
> +            .field("tm_min", &self.tm_min)
> +            .field("tm_hour", &self.tm_hour)
> +            .field("tm_mday", &self.tm_mday)
> +            .field("tm_mon", &self.tm_mon)
> +            .field("tm_year", &self.tm_year)
> +            .field("tm_wday", &self.tm_wday)
> +            .field("tm_yday", &self.tm_yday)
> +            .field("tm_isdst", &self.tm_isdst)
> +            .field("tm_gmtoff", &self.tm_gmtoff)
> +            .field("tm_zone", &self.tm_zone)
> +            .finish()
> +    }
> +}
> +
> +/// These are now exposed via `libc`, but they're part of glibc.
> +mod imports {
> +    extern "C" {
> +        pub fn strftime(
> +            s: *mut libc::c_char,
> +            max: libc::size_t,
> +            format: *const libc::c_char,
> +            tm: *const libc::tm,
> +        ) -> libc::size_t;
> +
> +        pub fn strptime(
> +            s: *const libc::c_char,
> +            format: *const libc::c_char,
> +            tm: *mut libc::tm,
> +        ) -> *const libc::c_char;
> +    }
> +}
> +
> +impl Tm {
> +    /// A zero-initialized time struct.
> +    #[inline(always)]
> +    pub fn zero() -> Self {
> +        unsafe { std::mem::zeroed() }
> +    }
> +
> +    /// Get the current time in the local timezone.
> +    pub fn now_local() -> Result<Self, Error> {
> +        Self::at_local(unsafe { libc::time(std::ptr::null_mut()) })
> +    }
> +
> +    /// Get a local time from a unix time stamp.
> +    pub fn at_local(time: libc::time_t) -> Result<Self, Error> {
> +        let mut out = MaybeUninit::<libc::tm>::uninit();
> +        Ok(Self(unsafe {
> +            if libc::localtime_r(&time, out.as_mut_ptr()).is_null() {
> +                bail!("failed to convert timestamp to local time");
> +            }
> +            out.assume_init()
> +        }))
> +    }
> +
> +    /// Assume this is an UTC time and convert it to a unix time stamp.
> +    ///
> +    /// Equivalent to `timegm(3)`, the gmt equivalent of `mktime(3)`.
> +    pub fn as_utc_to_epoch(&self) -> libc::time_t {
> +        let mut year = self.0.tm_year as i64 + 1900;
> +        let mon = self.0.tm_mon;
> +
> +        let mut res: libc::time_t = (year - 1970) * 365 + CAL_MTOD[mon as usize];
> +
> +        if mon <= 1 {
> +            year -= 1;
> +        }
> +
> +        res += (year - 1968) / 4;
> +        res -= (year - 1900) / 100;
> +        res += (year - 1600) / 400;
> +
> +        res += (self.0.tm_mday - 1) as i64;
> +        res = res * 24 + self.0.tm_hour as i64;
> +        res = res * 60 + self.0.tm_min as i64;
> +        res = res * 60 + self.0.tm_sec as i64;
> +
> +        res
> +    }
> +}
> +
> +/// Wrapper around `strftime(3)` to format time strings.
> +pub fn strftime(format: &CStr, tm: &Tm) -> Result<String, Error> {
> +    let mut buf = MaybeUninit::<[u8; 64]>::uninit();
> +
> +    let size = unsafe {
> +        imports::strftime(
> +            buf.as_mut_ptr() as *mut libc::c_char,
> +            64,
> +            format.as_ptr(),
> +            &tm.0,
> +        )
> +    };
> +    if size == 0 {
> +        bail!("failed to format time");
> +    }
> +    let size = size as usize;
> +
> +    let buf = unsafe { buf.assume_init() };
> +
> +    std::str::from_utf8(&buf[..size])
> +        .map_err(|_| format_err!("formatted time was not valid utf-8"))
> +        .map(str::to_string)
> +}
> +
> +/// Wrapper around `strptime(3)` to parse time strings.
> +pub fn strptime(time: &str, format: &CStr) -> Result<Tm, Error> {
> +    let mut out = MaybeUninit::<libc::tm>::uninit();
> +
> +    let time = CString::new(time).map_err(|_| format_err!("time string contains nul bytes"))?;
> +
> +    let end = unsafe {
> +        imports::strptime(
> +            time.as_ptr() as *const libc::c_char,
> +            format.as_ptr(),
> +            out.as_mut_ptr(),
> +        )
> +    };
> +
> +    if end.is_null() {
> +        bail!("failed to parse time string {:?}", time);
> +    }
> +
> +    Ok(Tm(unsafe { out.assume_init() }))
> +}





^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [pmg-devel] [PATCH log-tracker] drop 'time' dependency
  2022-01-04 13:57 [pmg-devel] [PATCH log-tracker] drop 'time' dependency Wolfgang Bumiller
  2022-01-05 19:01 ` Stoiko Ivanov
@ 2022-02-14 18:09 ` Stoiko Ivanov
  1 sibling, 0 replies; 3+ messages in thread
From: Stoiko Ivanov @ 2022-02-14 18:09 UTC (permalink / raw)
  To: Wolfgang Bumiller; +Cc: pmg-devel

applied - with the FIXME comment removed and my R-b, T-b tags added

huge thanks again!

On Tue,  4 Jan 2022 14:57:23 +0100
Wolfgang Bumiller <w.bumiller@proxmox.com> wrote:

> We're using a very old version of it and the functions we
> actually use are available in glibc anyway.
> 
> The only difference I found was that the result of
> glibc's `strptime()`'s `%s` does *not* want an additional
> `.to_local()` call anymore...
> 
> ... which was weird anyway.
> 
> As a minor optimization I pass the format string as `&CStr`
> to avoid the memcpy.
> 
> (The CString::new() call in `strptime()` only happens during
> parameter parsing)
> 
> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> ---
>  Cargo.toml  |   1 -
>  src/main.rs |  75 ++++++++---------------
>  src/time.rs | 169 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 194 insertions(+), 51 deletions(-)
>  create mode 100644 src/time.rs
> 
> .. snip ...




^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-02-14 18:09 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-04 13:57 [pmg-devel] [PATCH log-tracker] drop 'time' dependency Wolfgang Bumiller
2022-01-05 19:01 ` Stoiko Ivanov
2022-02-14 18:09 ` Stoiko Ivanov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal