Merge pull request thesofproject#95 from Rust-for-Linux/rust-document…

…-all

Add missing docs, enforce `missing-docs` and doc `compiler_builtins`

drivers/char/rust_example.rs

@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 
+//! Rust example module
+
 #![no_std]
 #![feature(allocator_api, global_asm)]
 #![feature(test)]

rust/Makefile

@@ -14,15 +14,18 @@ quiet_cmd_rustdoc = RUSTDOC $<
 	$(RUSTDOC) $(filter-out --emit=%, $(rustc_flags)) \
 		$(rustdoc_target_flags) -L $(objtree)/rust/ \
 		--output $(objtree)/rust/doc --crate-name $(subst rustdoc-,,$@) \
-		-Wmissing-docs @$(objtree)/include/generated/rustc_cfg $<
+		-Fmissing-docs @$(objtree)/include/generated/rustc_cfg $<
 
-rustdoc: rustdoc-module rustdoc-kernel
+rustdoc: rustdoc-module rustdoc-compiler_builtins rustdoc-kernel
 
 rustdoc-module: private rustdoc_target_flags = --crate-type proc-macro \
     --extern proc_macro
 rustdoc-module: $(srctree)/rust/module.rs FORCE
 	$(call if_changed,rustdoc)
 
+rustdoc-compiler_builtins: $(srctree)/rust/compiler_builtins.rs FORCE
+	$(call if_changed,rustdoc)
+
 rustdoc-kernel: private rustdoc_target_flags = --extern alloc \
     --extern module=$(objtree)/rust/libmodule.so
 rustdoc-kernel: $(srctree)/rust/kernel/lib.rs rustdoc-module \

rust/compiler_builtins.rs

@@ -1,22 +1,25 @@
 // SPDX-License-Identifier: GPL-2.0
 
-//! Our `compiler_builtins`.
+//! Our own `compiler_builtins`.
 //!
-//! Rust provides `compiler_builtins` as a port of LLVM's `compiler-rt`.
-//! Since we don't need the vast majority of them, we avoid the dependency
+//! Rust provides [`compiler_builtins`] as a port of LLVM's [`compiler-rt`].
+//! Since we do not need the vast majority of them, we avoid the dependency
 //! by providing this file.
 //!
-//! At the moment, some builtins are required that shouldn't be. For instance,
-//! `core` has floating-point functionality which we shouldn't be compiling in.
-//! For the moment, we define them to `panic!` at runtime for simplicity.
+//! At the moment, some builtins are required that should not be. For instance,
+//! [`core`] has floating-point functionality which we should not be compiling
+//! in. For the moment, we define them to [`panic!`] at runtime for simplicity.
 //! These are actually a superset of the ones we actually need to define,
 //! but it seems simpler to ban entire categories at once. In the future,
-//! we might be able to remove all this by providing our own custom `core` etc.,
-//! or perhaps `core` itself might provide `cfg` options to disable enough
-//! functionality to avoid requiring some of these.
+//! we might be able to remove all this by providing our own custom [`core`]
+//! etc., or perhaps [`core`] itself might provide `cfg` options to disable
+//! enough functionality to avoid requiring some of these.
 //!
-//! In any case, all these symbols are weakened to ensure we don't override
+//! In any case, all these symbols are weakened to ensure we do not override
 //! those that may be provided by the rest of the kernel.
+//!
+//! [`compiler_builtins`]: https://github.com/rust-lang/compiler-builtins
+//! [`compiler-rt`]: https://compiler-rt.llvm.org/
 
 #![feature(compiler_builtins)]
 #![compiler_builtins]
@@ -26,6 +29,7 @@
 macro_rules! define_panicking_intrinsics(
     ($reason: tt, { $($ident: ident, )* }) => {
         $(
+            #[doc(hidden)]
             #[no_mangle]
             pub extern "C" fn $ident() {
                 panic!($reason);

rust/kernel/allocator.rs

@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 
+//! Allocator support.
+
 use core::alloc::{GlobalAlloc, Layout};
 use core::ptr;
 
@@ -10,8 +12,8 @@ pub struct KernelAllocator;
 
 unsafe impl GlobalAlloc for KernelAllocator {
     unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
-        // krealloc is used instead of kmalloc because kmalloc is an inline function and can't be
-        // bound to as a result
+        // `krealloc()` is used instead of `kmalloc()` because the latter is
+        // an inline function and cannot be bound to as a result.
         bindings::krealloc(ptr::null(), layout.size(), bindings::GFP_KERNEL) as *mut u8
     }
 

rust/kernel/bindings.rs

@@ -1,5 +1,9 @@
 // SPDX-License-Identifier: GPL-2.0
 
+//! Bindings
+//!
+//! Imports the generated bindings by `bindgen`.
+
 #[allow(
     clippy::all,
     non_camel_case_types,

rust/kernel/bindings_helper.h

@@ -10,6 +10,6 @@
 #include <linux/version.h>
 #include <linux/miscdevice.h>
 
-// bindgen gets confused at certain things
+// `bindgen` gets confused at certain things
 const gfp_t BINDINGS_GFP_KERNEL = GFP_KERNEL;
 const gfp_t BINDINGS___GFP_ZERO = __GFP_ZERO;

rust/kernel/c_types.rs

@@ -1,67 +1,128 @@
 // SPDX-License-Identifier: GPL-2.0
 
+//! C types for the bindings.
+//!
+//! The bindings generated by `bindgen` use these types to map to the C ones.
+//!
+//! C's standard integer types may differ in width depending on
+//! the architecture, thus we need to conditionally compile those.
+
 #![allow(non_camel_case_types)]
 
 #[cfg(any(target_arch = "arm", target_arch = "x86"))]
 mod c {
+    /// C `void` type.
     pub type c_void = core::ffi::c_void;
 
+    /// C `char` type.
     pub type c_char = i8;
+
+    /// C `signed char` type.
     pub type c_schar = i8;
+
+    /// C `unsigned char` type.
     pub type c_uchar = u8;
 
+    /// C `short` type.
     pub type c_short = i16;
+
+    /// C `unsigned short` type.
     pub type c_ushort = u16;
 
+    /// C `int` type.
     pub type c_int = i32;
+
+    /// C `unsigned int` type.
     pub type c_uint = u32;
 
+    /// C `long` type.
     pub type c_long = i32;
+
+    /// C `unsigned long` type.
     pub type c_ulong = u32;
 
+    /// C `long long` type.
     pub type c_longlong = i64;
+
+    /// C `unsigned long long` type.
     pub type c_ulonglong = u64;
 
+    /// C `ssize_t` type (typically defined in `<sys/types.h>` by POSIX).
+    ///
+    /// For some 32-bit architectures like this one, the kernel defines it as
+    /// `int`, i.e. it is an [`i32`].
     pub type c_ssize_t = isize;
+
+    /// C `size_t` type (typically defined in `<stddef.h>`).
+    ///
+    /// For some 32-bit architectures like this one, the kernel defines it as
+    /// `unsigned int`, i.e. it is an [`u32`].
     pub type c_size_t = usize;
 }
 
 #[cfg(any(target_arch = "aarch64", target_arch = "x86_64"))]
 mod c {
+    /// C `void` type.
     pub type c_void = core::ffi::c_void;
 
+    /// C `char` type.
     pub type c_char = i8;
+
+    /// C `signed char` type.
     pub type c_schar = i8;
+
+    /// C `unsigned char` type.
     pub type c_uchar = u8;
 
+    /// C `short` type.
     pub type c_short = i16;
+
+    /// C `unsigned short` type.
     pub type c_ushort = u16;
 
+    /// C `int` type.
     pub type c_int = i32;
+
+    /// C `unsigned int` type.
     pub type c_uint = u32;
 
+    /// C `long` type.
     pub type c_long = i64;
+
+    /// C `unsigned long` type.
     pub type c_ulong = u64;
 
+    /// C `long long` type.
     pub type c_longlong = i64;
+
+    /// C `unsigned long long` type.
     pub type c_ulonglong = u64;
 
+    /// C `ssize_t` type (typically defined in `<sys/types.h>` by POSIX).
+    ///
+    /// For 64-bit architectures like this one, the kernel defines it as
+    /// `long`, i.e. it is an [`i64`].
     pub type c_ssize_t = isize;
+
+    /// C `size_t` type (typically defined in `<stddef.h>`).
+    ///
+    /// For 64-bit architectures like this one, the kernel defines it as
+    /// `unsigned long`, i.e. it is an [`u64`].
     pub type c_size_t = usize;
 }
 
 pub use c::*;
 
-/// Reads string until null byte is reached and returns slice excluding the terminating null.
+/// Reads string until null byte is reached and returns slice excluding the
+/// terminating null.
 ///
 /// # Safety
 ///
 /// The data from the pointer until the null terminator must be valid for reads
 /// and not mutated for all of `'a`. The length of the string must also be less
-/// than `isize::MAX`. See the documentation on [`from_raw_parts`] for further
-/// details on safety of converting a pointer to a slice.
-///
-/// [`from_raw_parts`]: https://doc.rust-lang.org/core/slice/fn.from_raw_parts.html
+/// than `isize::MAX`. See the documentation on
+/// [`core::slice::from_raw_parts()`] for further details on safety of
+/// converting a pointer to a slice.
 pub unsafe fn c_string_bytes<'a>(ptr: *const crate::c_types::c_char) -> &'a [u8] {
     let length = crate::bindings::strlen(ptr) as usize;
     &core::slice::from_raw_parts(ptr as *const u8, length)

rust/kernel/chrdev.rs

@@ -1,5 +1,13 @@
 // SPDX-License-Identifier: GPL-2.0
 
+//! Character devices.
+//!
+//! Also called "char devices", `chrdev`, `cdev`.
+//!
+//! C header: [`include/linux/cdev.h`](../../../../include/linux/cdev.h)
+//!
+//! Reference: <https://www.kernel.org/doc/html/latest/core-api/kernel-api.html#char-devices>
+
 use alloc::boxed::Box;
 use core::convert::TryInto;
 use core::marker::PhantomPinned;
@@ -19,8 +27,9 @@ struct RegistrationInner<const N: usize> {
     _pin: PhantomPinned,
 }
 
-/// chrdev registration. May contain up to a fixed number (`N`) of devices.
-/// Must be pinned.
+/// Character device registration.
+///
+/// May contain up to a fixed number (`N`) of devices. Must be pinned.
 pub struct Registration<const N: usize> {
     name: CStr<'static>,
     minors_start: u16,
@@ -29,6 +38,15 @@ pub struct Registration<const N: usize> {
 }
 
 impl<const N: usize> Registration<{ N }> {
+    /// Creates a [`Registration`] object for a character device.
+    ///
+    /// This does *not* register the device: see [`Self::register()`].
+    ///
+    /// This associated function is intended to be used when you need to avoid
+    /// a memory allocation, e.g. when the [`Registration`] is a member of
+    /// a bigger structure inside your [`crate::KernelModule`] instance. If you
+    /// are going to pin the registration right away, call
+    /// [`Self::new_pinned()`] instead.
     pub fn new(
         name: CStr<'static>,
         minors_start: u16,
@@ -42,6 +60,9 @@ impl<const N: usize> Registration<{ N }> {
         }
     }
 
+    /// Creates a pinned [`Registration`] object for a character device.
+    ///
+    /// This does *not* register the device: see [`Self::register()`].
     pub fn new_pinned(
         name: CStr<'static>,
         minors_start: u16,
@@ -53,15 +74,17 @@ impl<const N: usize> Registration<{ N }> {
             this_module,
         ))?))
     }
-    /// Register a character device with this range. Call this once per device
-    /// type (up to `N` times).
+
+    /// Registers a character device.
+    ///
+    /// You may call this once per device type, up to `N` times.
     pub fn register<T: file_operations::FileOperations>(self: Pin<&mut Self>) -> KernelResult<()> {
-        // SAFETY: we must ensure that we never move out of `this`.
+        // SAFETY: We must ensure that we never move out of `this`.
         let this = unsafe { self.get_unchecked_mut() };
         if this.inner.is_none() {
             let mut dev: bindings::dev_t = 0;
-            // SAFETY: Calling unsafe function. `this.name` has 'static
-            // lifetime
+            // SAFETY: Calling unsafe function. `this.name` has `'static`
+            // lifetime.
             let res = unsafe {
                 bindings::alloc_chrdev_region(
                     &mut dev,
@@ -86,7 +109,8 @@ impl<const N: usize> Registration<{ N }> {
             return Err(Error::EINVAL);
         }
         let cdev = inner.cdevs[inner.used].as_mut_ptr();
-        // SAFETY: calling unsafe functions and manipulating MaybeUninit ptr.
+        // SAFETY: Calling unsafe functions and manipulating `MaybeUninit`
+        // pointer.
         unsafe {
             bindings::cdev_init(cdev, &file_operations::FileOperationsVtable::<T>::VTABLE);
             (*cdev).owner = this.this_module.0;
@@ -100,14 +124,14 @@ impl<const N: usize> Registration<{ N }> {
     }
 }
 
-// SAFETY: `Registration` doesn't expose any of its state across threads (it's
-// fine for multiple threads to have a shared reference to it).
+// SAFETY: `Registration` does not expose any of its state across threads
+// (it is fine for multiple threads to have a shared reference to it).
 unsafe impl<const N: usize> Sync for Registration<{ N }> {}
 
 impl<const N: usize> Drop for Registration<{ N }> {
     fn drop(&mut self) {
         if let Some(inner) = self.inner.as_mut() {
-            // SAFETY: calling unsafe functions, `0..inner.used` of
+            // SAFETY: Calling unsafe functions, `0..inner.used` of
             // `inner.cdevs` are initialized in `Registration::register`.
             unsafe {
                 for i in 0..inner.used {