Mercurial > crates > nonstick

changeset 95:51c9d7e8261a

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Return owned strings rather than borrowed strings. It's going to be irritating to have to work with strings borrowed from the PAM handle rather than just using your own. They're cheap enough to copy.
author Paul Fisher <paul@pfish.zone>
date Mon, 23 Jun 2025 14:03:44 -0400
parents db167f96ba46
children f3e260f9ddcb
files src/handle.rs src/libpam/handle.rs src/libpam/memory.rs
diffstat 3 files changed, 21 insertions(+), 27 deletions(-) [+]
line wrap: on
line diff
--- a/src/handle.rs	Mon Jun 23 13:04:27 2025 -0400
+++ b/src/handle.rs	Mon Jun 23 14:03:44 2025 -0400
@@ -24,7 +24,7 @@
         /// [man]: https://www.man7.org/linux/man-pages/man3/pam_get_item.3.html
         /// [adg]: https://www.chiark.greenend.org.uk/doc/libpam-doc/html/adg-interface-by-app-expected.html#adg-pam_get_item
         /// [mwg]: https://www.chiark.greenend.org.uk/doc/libpam-doc/html/mwg-expected-by-module-item.html#mwg-pam_get_item
-        fn $getter(&self) -> Result<Option<&str>>;
+        fn $getter(&self) -> Result<Option<String>>;
     };
     ($(#[$md:meta])* set = $setter:ident, item = $item:literal $(, see = $see:path)?) => {
         $(#[$md])*
@@ -111,7 +111,7 @@
     ///
     /// [man]: https://www.man7.org/linux/man-pages/man3/pam_get_user.3.html
     /// [mwg]: https://www.chiark.greenend.org.uk/doc/libpam-doc/html/mwg-expected-by-module-item.html#mwg-pam_get_user
-    fn username(&mut self, prompt: Option<&str>) -> Result<&str>;
+    fn username(&mut self, prompt: Option<&str>) -> Result<String>;
 
     trait_item!(
         /// The identity of the user for whom service is being requested.
@@ -282,7 +282,7 @@
     ///
     /// [man]: https://www.man7.org/linux/man-pages/man3/pam_get_authtok.3.html
     /// [mwg]: https://www.chiark.greenend.org.uk/doc/libpam-doc/html/mwg-expected-by-module-item.html#mwg-pam_get_item
-    fn authtok(&mut self, prompt: Option<&str>) -> Result<&str>;
+    fn authtok(&mut self, prompt: Option<&str>) -> Result<String>;
 
     trait_item!(
         /// Gets the user's authentication token (e.g., password).
--- a/src/libpam/handle.rs	Mon Jun 23 13:04:27 2025 -0400
+++ b/src/libpam/handle.rs	Mon Jun 23 14:03:44 2025 -0400
@@ -65,7 +65,7 @@
 /// Macro to implement getting/setting a CStr-based item.
 macro_rules! cstr_item {
     (get = $getter:ident, item = $item_type:path) => {
-        fn $getter(&self) -> Result<Option<&str>> {
+        fn $getter(&self) -> Result<Option<String>> {
             unsafe { self.get_cstr_item($item_type) }
         }
     };
@@ -98,14 +98,14 @@
         }
     }
 
-    fn username(&mut self, prompt: Option<&str>) -> Result<&str> {
+    fn username(&mut self, prompt: Option<&str>) -> Result<String> {
         let prompt = memory::option_cstr(prompt)?;
         let mut output: *const c_char = ptr::null();
         let ret = unsafe {
             pam_ffi::pam_get_user(self, &mut output, memory::prompt_ptr(prompt.as_ref()))
         };
         ErrorCode::result_from(ret)?;
-        unsafe { memory::wrap_string(output) }
+        unsafe { memory::copy_pam_string(output) }
             .transpose()
             .unwrap_or(Err(ErrorCode::ConversationError))
     }
@@ -140,7 +140,7 @@
 }
 
 impl PamHandleModule for LibPamHandle {
-    fn authtok(&mut self, prompt: Option<&str>) -> Result<&str> {
+    fn authtok(&mut self, prompt: Option<&str>) -> Result<String> {
         let prompt = memory::option_cstr(prompt)?;
         let mut output: *const c_char = ptr::null_mut();
         // SAFETY: We're calling this with known-good values.
@@ -154,7 +154,7 @@
         };
         ErrorCode::result_from(res)?;
         // SAFETY: We got this string from PAM.
-        unsafe { memory::wrap_string(output) }
+        unsafe { memory::copy_pam_string(output) }
             .transpose()
             .unwrap_or(Err(ErrorCode::ConversationError))
     }
@@ -179,11 +179,11 @@
     /// # Safety
     ///
     /// You better be requesting an item which is a C string.
-    unsafe fn get_cstr_item(&self, item_type: ItemType) -> Result<Option<&str>> {
+    unsafe fn get_cstr_item(&self, item_type: ItemType) -> Result<Option<String>> {
         let mut output = ptr::null();
         let ret = unsafe { pam_ffi::pam_get_item(self, item_type as c_int, &mut output) };
         ErrorCode::result_from(ret)?;
-        memory::wrap_string(output.cast())
+        memory::copy_pam_string(output.cast())
     }
 
     /// Sets a C string item.
@@ -235,7 +235,7 @@
         }
     };
     (get = $get:ident$(, set = $set:ident)?) => {
-        delegate!(fn $get(&self) -> Result<Option<&str>>);
+        delegate!(fn $get(&self) -> Result<Option<String>>);
         $(delegate!(set = $set);)?
     };
     (set = $set:ident) => {
@@ -251,7 +251,7 @@
     fn log(&self, level: Level, entry: &str) {
         self.handle.log(level, entry)
     }
-    delegate!(fn username(&mut self, prompt: Option<&str>) -> Result<&str>);
+    delegate!(fn username(&mut self, prompt: Option<&str>) -> Result<String>);
     delegate!(get = user_item, set = set_user_item);
     delegate!(get = service, set = set_service);
     delegate!(get = user_prompt, set = set_user_prompt);
--- a/src/libpam/memory.rs	Mon Jun 23 13:04:27 2025 -0400
+++ b/src/libpam/memory.rs	Mon Jun 23 14:03:44 2025 -0400
@@ -52,22 +52,16 @@
 /// # Safety
 ///
 /// It's on you to provide a valid string.
-pub unsafe fn copy_pam_string(result_ptr: *const c_char) -> Result<String> {
-    Ok(wrap_string(result_ptr)?
-        .map(String::from)
-        .unwrap_or_default())
-}
-
-/// Wraps a string returned from PAM as an `Option<&str>`.
-pub unsafe fn wrap_string<'a>(data: *const c_char) -> Result<Option<&'a str>> {
-    match NonNull::new(data.cast_mut()) {
-        Some(data) => Ok(Some(
+pub unsafe fn copy_pam_string(result_ptr: *const c_char) -> Result<Option<String>> {
+    let borrowed = match NonNull::new(result_ptr.cast_mut()) {
+        Some(data) => Some(
             CStr::from_ptr(data.as_ptr())
                 .to_str()
                 .map_err(|_| ErrorCode::ConversationError)?,
-        )),
-        None => Ok(None),
-    }
+        ),
+        None => return Ok(None),
+    };
+    Ok(borrowed.map(String::from))
 }
 
 /// Allocates a string with the given contents on the C heap.
@@ -192,11 +186,11 @@
         malloc_str("hell\0 there").unwrap_err();
         unsafe {
             let copied = copy_pam_string(str).unwrap();
-            assert_eq!("hello there", copied);
+            assert_eq!("hello there", copied.unwrap());
             zero_c_string(str);
             let idx_three = str.add(3).as_mut().unwrap();
             *idx_three = 0x80u8 as i8;
-            let zeroed = copy_pam_string(str).unwrap();
+            let zeroed = copy_pam_string(str).unwrap().unwrap();
             assert!(zeroed.is_empty());
             free(str);
         }