diff src/libpam/environ.rs @ 141:a508a69c068a

Remove a lot of Results from functions. Many functions are documented to only return failing Results when given improper inputs or when there is a memory allocation failure (which can be verified by looking at the source). In cases where we know our input is correct, we don't need to check for memory allocation errors for the same reason that Rust doesn't do so when you, e.g., create a new Vec.
author Paul Fisher <paul@pfish.zone>
date Sat, 05 Jul 2025 17:16:56 -0400
parents 33b9622ed6d2
children ebb71a412b58
line wrap: on
line diff
--- a/src/libpam/environ.rs	Sat Jul 05 17:11:33 2025 -0400
+++ b/src/libpam/environ.rs	Sat Jul 05 17:16:56 2025 -0400
@@ -1,34 +1,33 @@
-use crate::constants::{ErrorCode, Result};
 use crate::environ::{EnvironMap, EnvironMapMut};
-use crate::libpam::memory::CHeapString;
-use crate::libpam::{memory, LibPamHandle};
+use crate::libpam::memory::{CHeapBox, CHeapString};
+use crate::libpam::{memory, RawPamHandle};
 use std::ffi::{c_char, CStr, CString, OsStr, OsString};
 use std::marker::PhantomData;
 use std::os::unix::ffi::{OsStrExt, OsStringExt};
 use std::ptr;
 use std::ptr::NonNull;
 
-pub struct LibPamEnviron<'a> {
-    source: &'a LibPamHandle,
-}
-
-pub struct LibPamEnvironMut<'a> {
-    source: &'a mut LibPamHandle,
-}
-
-impl LibPamHandle {
+impl RawPamHandle {
     fn environ_get(&self, key: &OsStr) -> Option<OsString> {
         let key = CString::new(key.as_bytes()).ok()?;
         // SAFETY: We are a valid handle and are calling with a good key.
-        unsafe { copy_env(libpam_sys::pam_getenv(self.0.as_ref(), key.as_ptr())) }
+        let src = unsafe { libpam_sys::pam_getenv(self.raw_ref(), key.as_ptr()) };
+        let val = match NonNull::new(src) {
+            None => return None,
+            Some(ptr) => ptr.as_ptr(),
+        };
+        // SAFETY: We were just returned this string from PAM.
+        // We have to trust it.
+        let c_str = unsafe { CStr::from_ptr(val) };
+        Some(OsString::from_vec(c_str.to_bytes().to_vec()))
     }
 
-    fn environ_set(&mut self, key: &OsStr, value: Option<&OsStr>) -> Result<Option<OsString>> {
+    fn environ_set(&mut self, key: &OsStr, value: Option<&OsStr>) -> Option<OsString> {
         let old = self.environ_get(key);
         if old.is_none() && value.is_none() {
             // pam_putenv returns an error if we try to remove a non-existent
             // environment variable, so just avoid that entirely.
-            return Ok(None);
+            return None;
         }
         let total_len = key.len() + value.map(OsStr::len).unwrap_or_default() + 2;
         let mut result = Vec::with_capacity(total_len);
@@ -37,43 +36,41 @@
             result.push(b'=');
             result.extend(value.as_bytes());
         }
-        let put = CString::new(result).map_err(|_| ErrorCode::ConversationError)?;
+        let put = CString::new(result).unwrap();
         // SAFETY: This is a valid handle and a valid environment string.
-        ErrorCode::result_from(unsafe { libpam_sys::pam_putenv(self.0.as_mut(), put.as_ptr()) })?;
-        Ok(old)
+        // pam_putenv is only ever going to
+        let _ = unsafe { libpam_sys::pam_putenv(self.raw_mut(), put.as_ptr()) };
+        old
     }
 
-    fn environ_iter(&self) -> Result<impl Iterator<Item = (OsString, OsString)>> {
+    fn environ_iter(&self) -> impl Iterator<Item = (OsString, OsString)> {
         // SAFETY: This is a valid PAM handle. It will return valid data.
         unsafe {
-            NonNull::new(libpam_sys::pam_getenvlist(self.0.as_ref()))
+            NonNull::new(libpam_sys::pam_getenvlist(self.raw_ref()))
                 .map(|ptr| EnvList::from_ptr(ptr.cast()))
-                .ok_or(ErrorCode::BufferError)
+                .unwrap_or_else(EnvList::empty)
         }
     }
 }
 
-/// Copies the data of the given C string pointer to an OsString,
-/// or None if src is null.
-unsafe fn copy_env(src: *const c_char) -> Option<OsString> {
-    let val = match NonNull::new(src.cast_mut()) {
-        None => return None,
-        Some(ptr) => ptr.as_ptr(),
-    };
-    // SAFETY: We were just returned this string from PAM.
-    // We have to trust it.
-    let c_str = unsafe { CStr::from_ptr(val) };
-    Some(OsString::from_vec(c_str.to_bytes().to_vec()))
+/// A view to the environment stored in a PAM handle.
+pub struct LibPamEnviron<'a> {
+    source: &'a RawPamHandle,
+}
+
+/// A mutable view to the environment stored in a PAM handle.
+pub struct LibPamEnvironMut<'a> {
+    source: &'a mut RawPamHandle,
 }
 
 impl<'a> LibPamEnviron<'a> {
-    pub fn new(source: &'a LibPamHandle) -> Self {
+    pub fn new(source: &'a RawPamHandle) -> Self {
         Self { source }
     }
 }
 
 impl<'a> LibPamEnvironMut<'a> {
-    pub fn new(source: &'a mut LibPamHandle) -> Self {
+    pub fn new(source: &'a mut RawPamHandle) -> Self {
         Self { source }
     }
 }
@@ -83,7 +80,7 @@
         self.source.environ_get(key.as_ref())
     }
 
-    fn iter(&self) -> Result<impl Iterator<Item = (OsString, OsString)>> {
+    fn iter(&self) -> impl Iterator<Item = (OsString, OsString)> {
         self.source.environ_iter()
     }
 }
@@ -93,21 +90,17 @@
         self.source.environ_get(key.as_ref())
     }
 
-    fn iter(&self) -> Result<impl Iterator<Item = (OsString, OsString)>> {
+    fn iter(&self) -> impl Iterator<Item = (OsString, OsString)> {
         self.source.environ_iter()
     }
 }
 
 impl EnvironMapMut<'_> for LibPamEnvironMut<'_> {
-    fn insert(
-        &mut self,
-        key: impl AsRef<OsStr>,
-        val: impl AsRef<OsStr>,
-    ) -> Result<Option<OsString>> {
+    fn insert(&mut self, key: impl AsRef<OsStr>, val: impl AsRef<OsStr>) -> Option<OsString> {
         self.source.environ_set(key.as_ref(), Some(val.as_ref()))
     }
 
-    fn remove(&mut self, key: impl AsRef<OsStr>) -> Result<Option<OsString>> {
+    fn remove(&mut self, key: impl AsRef<OsStr>) -> Option<OsString> {
         self.source.environ_set(key.as_ref(), None)
     }
 }
@@ -120,10 +113,19 @@
     start: NonNull<Option<EnvVar>>,
     /// The environment variable we're about to iterate into.
     current: NonNull<Option<EnvVar>>,
-    _owner: PhantomData<&'a LibPamHandle>,
+    _owner: PhantomData<&'a RawPamHandle>,
 }
 
 impl EnvList<'_> {
+    fn empty() -> Self {
+        let none: CHeapBox<Option<EnvVar>> = CHeapBox::new(None);
+        let ptr = CHeapBox::into_ptr(none);
+        Self {
+            start: ptr,
+            current: ptr,
+            _owner: PhantomData,
+        }
+    }
     unsafe fn from_ptr(ptr: NonNull<*mut c_char>) -> Self {
         Self {
             start: ptr.cast(),