# HG changeset patch
# User Paul Fisher <paul@pfish.zone>
# Date 1750803098 14400
# Node ID 94eb11cb1798df6cfb86d3c40c0ee7bf0c1e8c18
# Parent  94b51fa4f797c490b4bac7793884a0c494cc74f1
Improve documentation for pam_start.

diff -r 94b51fa4f797 -r 94eb11cb1798 src/libpam/handle.rs
--- a/src/libpam/handle.rs	Tue Jun 24 17:54:33 2025 -0400
+++ b/src/libpam/handle.rs	Tue Jun 24 18:11:38 2025 -0400
@@ -37,6 +37,12 @@
     /// The last return value from the handle.
     last_return: Cell<Result<()>>,
     /// If set, the Conversation that this PAM handle owns.
+    ///
+    /// We have to hold on to this because the PAM specification doesn't
+    /// actually say what the PAM library should do with a passed-in
+    /// conversation. Linux-PAM copies the contents of the `pam_conv` struct
+    /// that you pass in to `pam_start`, but OpenPAM uses the pointer itself,
+    /// so you have to keep it in one place.
     conversation: Option<Box<LibPamConversation<'a>>>,
 }
 
@@ -47,33 +53,40 @@
 }
 
 impl HandleBuilder {
-    /// Creates a new HandleBuilder for the given service.
-    fn new(service_name: String) -> Self {
-        Self {
-            service_name,
-            username: Default::default(),
-        }
-    }
     /// Updates the service name.
     pub fn service_name(mut self, service_name: String) -> Self {
         self.service_name = service_name;
         self
     }
-    /// Updates the username.
+    /// Sets the username. Setting this will avoid the need for an extra
+    /// round trip through the conversation and may otherwise improve
+    /// the login experience.
     pub fn username(mut self, username: String) -> Self {
         self.username = Some(username);
         self
     }
-
+    /// Builds a PAM handle and starts the transaction.
     pub fn build(self, conv: &impl Conversation) -> Result<OwnedLibPamHandle> {
         OwnedLibPamHandle::start(self.service_name, self.username, conv)
     }
 }
 
 impl OwnedLibPamHandle<'_> {
+    /// Creates a builder to start a PAM transaction for the given service.
+    ///
+    /// The service name is what controls the steps and checks PAM goes through
+    /// when authenticating a user. This corresponds to the configuration file
+    /// named <code>/etc/pam.d/<var>service_name</var></code>.
+    ///
+    /// For more information, see the [`pam_start` man page][man], or
+    /// [`pam_start` in the PAM Application Developers' Guide][adg].
+    ///
+    /// [man]: https://www.man7.org/linux/man-pages/man3/pam_start.3.html
+    /// [adg]: https://www.chiark.greenend.org.uk/doc/libpam-doc/html/adg-interface-by-app-expected.html#adg-pam_start
     pub fn build_with_service(service_name: String) -> HandleBuilder {
-        HandleBuilder::new(service_name)
+        HandleBuilder { service_name, username: None }
     }
+
     fn start(
         service_name: String,
         username: Option<String>,
diff -r 94b51fa4f797 -r 94eb11cb1798 src/libpam/pam_ffi.rs
--- a/src/libpam/pam_ffi.rs	Tue Jun 24 17:54:33 2025 -0400
+++ b/src/libpam/pam_ffi.rs	Tue Jun 24 18:11:38 2025 -0400
@@ -30,8 +30,8 @@
 pub struct Answer {
     /// Owned pointer to the data returned in an answer.
     /// For most answers, this will be a [`CHeapString`],
-    /// but for [`BinaryQAndA`](crate::conv::BinaryQAndA)s (a Linux-PAM extension),
-    /// this will be a [`CHeapBox`] of
+    /// but for [`BinaryQAndA`](crate::conv::BinaryQAndA)s
+    /// (a Linux-PAM extension), this will be a [`CHeapBox`] of
     /// [`CBinaryData`](crate::libpam::memory::CBinaryData).
     pub data: Option<CHeapBox<c_void>>,
     /// Unused. Just here for the padding.