all lists on 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal