changeset 157:0099f2f79f86

Switch logging interface to accept fmt::Arguments. This means that we don't have to format arguments eagerly when logging; an implementation could choose to discard them if it wanted to, avoiding allocations and expensive format calls.
author Paul Fisher <paul@pfish.zone>
date Wed, 09 Jul 2025 16:59:30 -0400
parents 66e662cde087
children d5b7b28d754e
files src/handle.rs src/libpam/handle.rs src/logging.rs
diffstat 3 files changed, 25 insertions(+), 18 deletions(-) [+]
line wrap: on
line diff
--- a/src/handle.rs	Tue Jul 08 01:04:30 2025 -0400
+++ b/src/handle.rs	Wed Jul 09 16:59:30 2025 -0400
@@ -7,6 +7,7 @@
 use crate::items::{getter, Items, ItemsMut};
 use crate::logging::{Level, Location};
 use std::ffi::{OsStr, OsString};
+use std::fmt;
 
 /// Functionality for both PAM applications and PAM modules.
 ///
@@ -44,12 +45,12 @@
     /// nonstick::info!(pam_hdl, "using network backend");
     /// nonstick::debug!(pam_hdl, "sending GET request to {url}");
     /// // But if you really want to, you can call this yourself:
-    /// pam_hdl.log(Level::Warning, location!(), "this is unnecessarily verbose");
+    /// pam_hdl.log(Level::Warn, location!(), format_args!("this is unnecessarily verbose"));
     /// # }
     /// ```
     #[doc = man7!(3 pam_syslog)]
     #[doc = manbsd!(3 openpam_log)]
-    fn log(&self, level: Level, loc: Location<'_>, entry: &str);
+    fn log(&self, level: Level, loc: Location<'_>, entry: fmt::Arguments);
 
     /// Retrieves the name of the user who is authenticating or logging in.
     ///
--- a/src/libpam/handle.rs	Tue Jul 08 01:04:30 2025 -0400
+++ b/src/libpam/handle.rs	Wed Jul 09 16:59:30 2025 -0400
@@ -17,8 +17,8 @@
 use std::ffi::{c_char, c_int, c_void, CString, OsStr, OsString};
 use std::mem::ManuallyDrop;
 use std::os::unix::ffi::OsStrExt;
-use std::ptr;
 use std::ptr::NonNull;
+use std::{fmt, ptr};
 
 /// An owned PAM handle.
 pub struct LibPamTransaction<C: Conversation> {
@@ -223,7 +223,7 @@
 }
 
 impl<C: Conversation> PamShared for LibPamTransaction<C> {
-    delegate!(fn log(&self, level: Level, location: Location<'_>, entry: &str) -> ());
+    delegate!(fn log(&self, level: Level, location: Location<'_>, entry: fmt::Arguments) -> ());
     delegate!(fn environ(&self) -> impl EnvironMap);
     delegate!(fn environ_mut(&mut self) -> impl EnvironMapMut);
     delegate!(fn username(&mut self, prompt: Option<&OsStr>) -> Result<OsString>);
@@ -322,23 +322,28 @@
 }
 
 impl PamShared for LibPamHandle {
-    fn log(&self, level: Level, loc: Location<'_>, entry: &str) {
-        let entry = match CString::new(entry).or_else(|_| CString::new(dbg!(entry))) {
-            Ok(cstr) => cstr,
-            _ => return,
+    fn log(&self, level: Level, loc: Location<'_>, entry: fmt::Arguments) {
+        let entry = match CString::new(entry.to_string()).ok() {
+            Some(e) => e,
+            None => return,
         };
         #[cfg(pam_impl = "LinuxPam")]
         {
             let level = match level {
                 Level::Error => libc::LOG_ERR,
-                Level::Warning => libc::LOG_WARNING,
+                Level::Warn => libc::LOG_WARNING,
                 Level::Info => libc::LOG_INFO,
                 Level::Debug => libc::LOG_DEBUG,
             };
             _ = loc;
             // SAFETY: We're calling this function with a known value.
             unsafe {
-                libpam_sys::pam_syslog(self.raw_ref(), level, "%s\0".as_ptr().cast(), entry.as_ptr())
+                libpam_sys::pam_syslog(
+                    self.raw_ref(),
+                    level,
+                    b"%s\0".as_ptr().cast(),
+                    entry.as_ptr(),
+                )
             }
         }
         #[cfg(pam_impl = "OpenPam")]
@@ -346,7 +351,7 @@
             let func = CString::new(loc.function).unwrap_or(CString::default());
             let level = match level {
                 Level::Error => libpam_sys::PAM_LOG_ERROR,
-                Level::Warning => libpam_sys::PAM_LOG_NOTICE,
+                Level::Warn => libpam_sys::PAM_LOG_NOTICE,
                 Level::Info => libpam_sys::PAM_LOG_VERBOSE,
                 Level::Debug => libpam_sys::PAM_LOG_DEBUG,
             };
@@ -355,7 +360,7 @@
                 libpam_sys::_openpam_log(
                     level as c_int,
                     func.as_ptr(),
-                    "%s\0".as_ptr().cast(),
+                    b"%s\0".as_ptr().cast(),
                     entry.as_ptr(),
                 )
             }
--- a/src/logging.rs	Tue Jul 08 01:04:30 2025 -0400
+++ b/src/logging.rs	Wed Jul 09 16:59:30 2025 -0400
@@ -25,7 +25,7 @@
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
 pub enum Level {
     Error,
-    Warning,
+    Warn,
     Info,
     Debug,
 }
@@ -64,7 +64,7 @@
 #[macro_export]
 macro_rules! __log_internal {
     ($handle:expr, $level:ident, $($arg:tt)+) => {
-        $handle.log($crate::logging::Level::$level, $crate::location!(), &format!($($arg)+));
+        $handle.log($crate::logging::Level::$level, $crate::location!(), format_args!($($arg)+));
     }
 }
 
@@ -122,7 +122,7 @@
 /// # }
 /// ```
 #[macro_export]
-macro_rules! warn { ($handle:expr, $($arg:tt)+) => { $crate::__log_internal!($handle, Warning, $($arg)+);}}
+macro_rules! warn { ($handle:expr, $($arg:tt)+) => { $crate::__log_internal!($handle, Warn, $($arg)+);}}
 
 /// Logs a message at info level via the given PAM handle.
 ///
@@ -163,14 +163,15 @@
 mod tests {
     use super::*;
     use std::cell::RefCell;
+    use std::fmt;
 
     #[test]
     fn test_logging() {
         struct Logger(RefCell<Vec<(Level, String)>>);
 
         impl Logger {
-            fn log(&self, level: Level, _: Location<'_>, text: &str) {
-                self.0.borrow_mut().push((level, text.to_owned()))
+            fn log(&self, level: Level, _: Location<'_>, text: fmt::Arguments) {
+                self.0.borrow_mut().push((level, text.to_string()))
             }
         }
 
@@ -187,7 +188,7 @@
         assert_eq!(
             vec![
                 (Level::Error, "here is another thing: 99".to_owned()),
-                (Level::Warning, "watch out!".to_owned()),
+                (Level::Warn, "watch out!".to_owned()),
                 (Level::Info, "here is some info: information".to_owned()),
                 (Level::Debug, "here is something: Error".to_owned()),
             ],