Make rustdoc rendered output better (rustls#156)

Previously, rustdoc didn't render most of our types and functions,
because they weren't brought in with `pub mod` or `pub use`. This change
makes everything public that should be. Also, some internal-only things
were accidentally public; made those pub(crate).

Wrapped most method-like functions in an `impl` block, so they become
associated functions and are listed on their struct's page rather than
on separate pages by themselves.

Hide all macros, because they are internal implementation details.

Fix some leftover places where comments referred to
`rustls_client_session` or `rustls_server_session`.

This change doesn't affect visibility or naming of symbols in the
built C library, just how rustdoc renders things.

Note: rustdoc probably isn't the best overall tool for documenting
this library, since it presents function signatures with Rust syntax and
types rather than C syntax and types. However, it's handy for navigation
and seeing how things fit together.

CHANGELOG.md

@@ -38,7 +38,7 @@
 
 ### Fixed
 
- - rustls_connect_get_peer_certificate was returning a dangling pointer.
+ - rustls_connection_get_peer_certificate was returning a dangling pointer.
    This is now fixed by having it return a reference that lives as long
    as the connection does. (#103)
 

src/crustls.h

@@ -125,7 +125,7 @@ typedef struct rustls_client_cert_verifier rustls_client_cert_verifier;
  * does not offer a certificate, the connection will succeed.
  *
  * The application can retrieve the certificate, if any, with
- * rustls_server_session_get_peer_certificate.
+ * rustls_connection_peer_certificate.
  */
 typedef struct rustls_client_cert_verifier_optional rustls_client_cert_verifier_optional;
 
@@ -302,7 +302,7 @@ typedef void (*rustls_log_callback)(void *userdata, const struct rustls_log_para
 typedef int rustls_io_result;
 
 /**
- * A callback for rustls_server_session_read_tls or rustls_client_session_read_tls.
+ * A callback for rustls_connection_read_tls.
  * An implementation of this callback should attempt to read up to n bytes from the
  * network, storing them in `buf`. If any bytes were stored, the implementation should
  * set out_n to the number of bytes stored and return 0. If there was an error,
@@ -311,23 +311,23 @@ typedef int rustls_io_result;
  * On other systems, any appropriate error code works.
  * It's best to make one read attempt to the network per call. Additional reads will
  * be triggered by subsequent calls to one of the `_read_tls` methods.
- * `userdata` is set to the value provided to `rustls_*_session_set_userdata`. In most
+ * `userdata` is set to the value provided to `rustls_connection_set_userdata`. In most
  * cases that should be a struct that contains, at a minimum, a file descriptor.
  * The buf and out_n pointers are borrowed and should not be retained across calls.
  */
 typedef rustls_io_result (*rustls_read_callback)(void *userdata, uint8_t *buf, size_t n, size_t *out_n);
 
 /**
- * A callback for rustls_server_session_write_tls or rustls_client_session_write_tls.
+ * A callback for rustls_connection_write_tls.
  * An implementation of this callback should attempt to write the `n` bytes in buf
  * to the network. If any bytes were written, the implementation should
  * set out_n to the number of bytes stored and return 0. If there was an error,
  * the implementation should return a nonzero rustls_io_result, which will be
  * passed through to the caller. On POSIX systems, returning `errno` is convenient.
  * On other systems, any appropriate error code works.
  * It's best to make one write attempt to the network per call. Additional writes will
- * be triggered by subsequent calls to one of the `_write_tls` methods.
- * `userdata` is set to the value provided to `rustls_*_session_set_userdata`. In most
+ * be triggered by subsequent calls to rustls_connection_write_tls.
+ * `userdata` is set to the value provided to `rustls_connection_set_userdata`. In most
  * cases that should be a struct that contains, at a minimum, a file descriptor.
  * The buf and out_n pointers are borrowed and should not be retained across calls.
  */
@@ -967,8 +967,8 @@ enum rustls_result rustls_connection_read(struct rustls_connection *conn,
 void rustls_connection_free(struct rustls_connection *conn);
 
 /**
- * After a rustls_client_session method returns an error, you may call
- * this method to get a pointer to a buffer containing a detailed error
+ * After a rustls function returns an error, you may call
+ * this to get a pointer to a buffer containing a detailed error
  * message. The contents of the error buffer will be out_n bytes long,
  * UTF-8 encoded, and not NUL-terminated.
  */

src/error.rs

@@ -11,49 +11,51 @@ use rustls::Error;
 #[repr(transparent)]
 pub struct rustls_io_result(pub libc::c_int);
 
-/// After a rustls_client_session method returns an error, you may call
-/// this method to get a pointer to a buffer containing a detailed error
-/// message. The contents of the error buffer will be out_n bytes long,
-/// UTF-8 encoded, and not NUL-terminated.
-#[no_mangle]
-pub extern "C" fn rustls_error(
-    result: rustls_result,
-    buf: *mut c_char,
-    len: size_t,
-    out_n: *mut size_t,
-) {
-    ffi_panic_boundary! {
-        let write_buf: &mut [u8] = unsafe {
-            let out_n: &mut size_t = match out_n.as_mut() {
-                Some(out_n) => out_n,
-                None => return,
+impl rustls_result {
+    /// After a rustls function returns an error, you may call
+    /// this to get a pointer to a buffer containing a detailed error
+    /// message. The contents of the error buffer will be out_n bytes long,
+    /// UTF-8 encoded, and not NUL-terminated.
+    #[no_mangle]
+    pub extern "C" fn rustls_error(
+        result: rustls_result,
+        buf: *mut c_char,
+        len: size_t,
+        out_n: *mut size_t,
+    ) {
+        ffi_panic_boundary! {
+            let write_buf: &mut [u8] = unsafe {
+                let out_n: &mut size_t = match out_n.as_mut() {
+                    Some(out_n) => out_n,
+                    None => return,
+                };
+                *out_n = 0;
+                if buf.is_null() {
+                    return;
+                }
+                slice::from_raw_parts_mut(buf as *mut u8, len as usize)
             };
-            *out_n = 0;
-            if buf.is_null() {
-                return;
+            let error_str = result.to_string();
+            let len: usize = min(write_buf.len() - 1, error_str.len());
+            write_buf[..len].copy_from_slice(&error_str.as_bytes()[..len]);
+            unsafe {
+                *out_n = len;
             }
-            slice::from_raw_parts_mut(buf as *mut u8, len as usize)
-        };
-        let error_str = result.to_string();
-        let len: usize = min(write_buf.len() - 1, error_str.len());
-        write_buf[..len].copy_from_slice(&error_str.as_bytes()[..len]);
-        unsafe {
-            *out_n = len;
         }
     }
-}
 
-#[no_mangle]
-pub extern "C" fn rustls_result_is_cert_error(result: rustls_result) -> bool {
-    match result_to_error(&result) {
-        Either::Error(
-            Error::InvalidCertificateData(_)
-            | Error::InvalidCertificateEncoding
-            | Error::InvalidCertificateSignature
-            | Error::InvalidCertificateSignatureType
-            | Error::InvalidSct(_),
-        ) => true,
-        _ => false,
+    #[no_mangle]
+    pub extern "C" fn rustls_result_is_cert_error(result: rustls_result) -> bool {
+        match result_to_error(&result) {
+            Either::Error(
+                Error::InvalidCertificateData(_)
+                | Error::InvalidCertificateEncoding
+                | Error::InvalidCertificateSignature
+                | Error::InvalidCertificateSignatureType
+                | Error::InvalidSct(_),
+            ) => true,
+            _ => false,
+        }
     }
 }
 

src/io.rs

@@ -6,7 +6,7 @@ use crate::error::rustls_io_result;
 use crate::rslice::rustls_slice_bytes;
 use std::ops::Deref;
 
-/// A callback for rustls_server_session_read_tls or rustls_client_session_read_tls.
+/// A callback for rustls_connection_read_tls.
 /// An implementation of this callback should attempt to read up to n bytes from the
 /// network, storing them in `buf`. If any bytes were stored, the implementation should
 /// set out_n to the number of bytes stored and return 0. If there was an error,
@@ -15,7 +15,7 @@ use std::ops::Deref;
 /// On other systems, any appropriate error code works.
 /// It's best to make one read attempt to the network per call. Additional reads will
 /// be triggered by subsequent calls to one of the `_read_tls` methods.
-/// `userdata` is set to the value provided to `rustls_*_session_set_userdata`. In most
+/// `userdata` is set to the value provided to `rustls_connection_set_userdata`. In most
 /// cases that should be a struct that contains, at a minimum, a file descriptor.
 /// The buf and out_n pointers are borrowed and should not be retained across calls.
 pub type rustls_read_callback = Option<
@@ -51,16 +51,16 @@ impl Read for CallbackReader {
     }
 }
 
-/// A callback for rustls_server_session_write_tls or rustls_client_session_write_tls.
+/// A callback for rustls_connection_write_tls.
 /// An implementation of this callback should attempt to write the `n` bytes in buf
 /// to the network. If any bytes were written, the implementation should
 /// set out_n to the number of bytes stored and return 0. If there was an error,
 /// the implementation should return a nonzero rustls_io_result, which will be
 /// passed through to the caller. On POSIX systems, returning `errno` is convenient.
 /// On other systems, any appropriate error code works.
 /// It's best to make one write attempt to the network per call. Additional writes will
-/// be triggered by subsequent calls to one of the `_write_tls` methods.
-/// `userdata` is set to the value provided to `rustls_*_session_set_userdata`. In most
+/// be triggered by subsequent calls to rustls_connection_write_tls.
+/// `userdata` is set to the value provided to `rustls_connection_set_userdata`. In most
 /// cases that should be a struct that contains, at a minimum, a file descriptor.
 /// The buf and out_n pointers are borrowed and should not be retained across calls.
 pub type rustls_write_callback = Option<

src/lib.rs

@@ -6,17 +6,19 @@ use std::sync::Arc;
 use std::{cmp::min, thread::AccessError};
 use std::{mem, slice};
 
-mod cipher;
-mod client;
-mod connection;
-mod enums;
+pub mod cipher;
+pub mod client;
+pub mod connection;
+pub mod enums;
 mod error;
-mod io;
-mod log;
+pub mod io;
+pub mod log;
 mod panic;
-mod rslice;
-mod server;
-mod session;
+pub mod rslice;
+pub mod server;
+pub mod session;
+
+pub use error::rustls_result;
 
 use crate::log::rustls_log_callback;
 use crate::panic::PanicOrDefault;
@@ -32,10 +34,10 @@ use crate::panic::PanicOrDefault;
 // Rust code, we model these thread locals as a stack, so we can always
 // restore the previous version.
 thread_local! {
-    pub static USERDATA: RefCell<Vec<Userdata>> = RefCell::new(Vec::new());
+    pub(crate) static USERDATA: RefCell<Vec<Userdata>> = RefCell::new(Vec::new());
 }
 
-pub struct Userdata {
+pub(crate) struct Userdata {
     userdata: *mut c_void,
     log_callback: rustls_log_callback,
 }
@@ -47,7 +49,7 @@ pub struct Userdata {
 ///  - The top item on that stack must be the one this guard was built with.
 ///  - The `data` field must not be None.
 /// If any of these invariants fails, try_drop will return an error.
-pub struct UserdataGuard {
+pub(crate) struct UserdataGuard {
     // Keep a copy of the data we expect to be popping off the stack. This allows
     // us to check for consistency, and also serves to make this type !Send:
     // https://doc.rust-lang.org/nightly/std/primitive.pointer.html#impl-Send-1
@@ -104,7 +106,7 @@ impl Drop for UserdataGuard {
 }
 
 #[derive(Clone, Debug)]
-pub enum UserdataError {
+pub(crate) enum UserdataError {
     /// try_pop was called twice.
     AlreadyPopped,
     /// The RefCell is borrowed somewhere else.
@@ -119,7 +121,7 @@ pub enum UserdataError {
 }
 
 #[must_use = "If you drop the guard, userdata will be immediately cleared"]
-pub fn userdata_push(
+pub(crate) fn userdata_push(
     u: *mut c_void,
     cb: rustls_log_callback,
 ) -> Result<UserdataGuard, UserdataError> {
@@ -140,7 +142,7 @@ pub fn userdata_push(
     Ok(UserdataGuard::new(u))
 }
 
-pub fn userdata_get() -> Result<*mut c_void, UserdataError> {
+pub(crate) fn userdata_get() -> Result<*mut c_void, UserdataError> {
     USERDATA
         .try_with(|userdata| {
             userdata.try_borrow_mut().map_or_else(
@@ -154,7 +156,7 @@ pub fn userdata_get() -> Result<*mut c_void, UserdataError> {
         .unwrap_or(Err(UserdataError::AccessError))
 }
 
-pub fn log_callback_get() -> Result<(rustls_log_callback, *mut c_void), UserdataError> {
+pub(crate) fn log_callback_get() -> Result<(rustls_log_callback, *mut c_void), UserdataError> {
     USERDATA
         .try_with(|userdata| {
             userdata.try_borrow_mut().map_or_else(
@@ -274,8 +276,8 @@ mod tests {
 // Keep in sync with Cargo.toml.
 const RUSTLS_CRATE_VERSION: &str = "0.20.0";
 
-/// CastPtr represents the relationship between a snake case type (like rustls_client_session)
-/// and the corresponding Rust type (like ClientSession). For each matched pair of types, there
+/// CastPtr represents the relationship between a snake case type (like rustls_client_config)
+/// and the corresponding Rust type (like ClientConfig). For each matched pair of types, there
 /// should be an `impl CastPtr for foo_bar { RustTy = FooBar }`.
 ///
 /// This allows us to avoid using `as` in most places, and ensure that when we cast, we're
@@ -311,6 +313,7 @@ pub(crate) trait BoxCastPtr: CastPtr + Sized {
     }
 }
 
+#[doc(hidden)]
 #[macro_export]
 macro_rules! try_slice {
     ( $ptr:expr, $count:expr ) => {
@@ -322,6 +325,7 @@ macro_rules! try_slice {
     };
 }
 
+#[doc(hidden)]
 #[macro_export]
 macro_rules! try_mut_slice {
     ( $ptr:expr, $count:expr ) => {
@@ -365,6 +369,7 @@ impl CastPtr for *const u8 {
 /// based on the context;
 /// Example:
 ///   let config: &mut ClientConfig = try_ref_from_ptr!(builder);
+#[doc(hidden)]
 #[macro_export]
 macro_rules! try_ref_from_ptr {
     ( $var:ident ) => {
@@ -375,6 +380,7 @@ macro_rules! try_ref_from_ptr {
     };
 }
 
+#[doc(hidden)]
 #[macro_export]
 macro_rules! try_mut_from_ptr {
     ( $var:ident ) => {
@@ -385,6 +391,7 @@ macro_rules! try_mut_from_ptr {
     };
 }
 
+#[doc(hidden)]
 #[macro_export]
 macro_rules! try_callback {
     ( $var:ident ) => {
@@ -419,10 +426,9 @@ pub extern "C" fn rustls_version(buf: *mut c_char, len: size_t) -> size_t {
     }
 }
 
-/// In rustls_server_config_builder_build, and rustls_client_config_builder_build,
-/// we create an Arc, then call `into_raw` and return the resulting raw pointer
-/// to C. C can then call rustls_server_session_new multiple times using that
-/// same raw pointer. On each call, we need to reconstruct the Arc. But once we reconstruct the Arc,
+/// Sometimes we create an Arc, then call `into_raw` and return the resulting raw pointer
+/// to C. C can then call back into rustls multiple times using that same raw pointer.
+/// On each call, we need to reconstruct the Arc. But once we reconstruct the Arc,
 /// its reference count will be decremented on drop. We need to reference count to stay at 1,
 /// because the C code is holding a copy. This function turns the raw pointer back into an Arc,
 /// clones it to increment the reference count (which will make it 2 in this particular case), and

src/log.rs

@@ -35,7 +35,7 @@ pub(crate) fn ensure_log_registered() {
     log::set_max_level(log::LevelFilter::Debug)
 }
 
-type rustls_log_level = usize;
+pub type rustls_log_level = usize;
 
 /// Return a rustls_str containing the stringified version of a log level.
 #[no_mangle]
@@ -53,8 +53,8 @@ pub extern "C" fn rustls_log_level_str(level: rustls_log_level) -> rustls_str<'s
 
 #[repr(C)]
 pub struct rustls_log_params<'a> {
-    level: rustls_log_level,
-    message: rustls_str<'a>,
+    pub level: rustls_log_level,
+    pub message: rustls_str<'a>,
 }
 
 #[allow(non_camel_case_types)]

src/panic.rs

@@ -92,6 +92,7 @@ impl NullParameterOrDefault for rustls_io_result {
     }
 }
 
+#[doc(hidden)]
 #[macro_export]
 macro_rules! ffi_panic_boundary {
     ( $($tt:tt)* ) => {