Use Pin<T> in the chrdev API

This makes the safety more a part of the type-system. Also changes the chrdev API to a) require compile-time declaration of how many items it can have, b) allows gradual registration of chrdevs.

Fixes #53

Author: alex

rust/kernel/chrdev.rs

@@ -1,100 +1,106 @@
 // SPDX-License-Identifier: GPL-2.0
 
 use core::convert::TryInto;
-use core::mem;
-use core::ops::Range;
-
-use alloc::boxed::Box;
-use alloc::vec;
-use alloc::vec::Vec;
+use core::marker::PhantomPinned;
+use core::mem::MaybeUninit;
+use core::pin::Pin;
 
 use crate::bindings;
 use crate::c_types;
 use crate::error::{Error, KernelResult};
 use crate::file_operations;
 use crate::types::CStr;
 
-pub fn builder(name: CStr<'static>, minors: Range<u16>) -> KernelResult<Builder> {
-    Ok(Builder {
-        name,
-        minors,
-        file_ops: vec![],
-    })
+struct RegistrationInner<const N: usize> {
+    dev: bindings::dev_t,
+    used: usize,
+    cdevs: [MaybeUninit<bindings::cdev>; N],
+    _pin: PhantomPinned,
 }
 
-pub struct Builder {
+/// chrdev registration. May contain up to a fixed number (N) of devices. Must
+/// be pinned.
+pub struct Registration<const N: usize> {
     name: CStr<'static>,
-    minors: Range<u16>,
-    file_ops: Vec<&'static bindings::file_operations>,
+    minors_start: u16,
+    this_module: &'static crate::ThisModule,
+    inner: Option<RegistrationInner<N>>,
 }
 
-impl Builder {
-    pub fn register_device<T: file_operations::FileOperations>(mut self) -> Builder {
-        if self.file_ops.len() >= self.minors.len() {
-            panic!("More devices registered than minor numbers allocated.")
+impl<const N: usize> Registration<{ N }> {
+    pub fn new(
+        name: CStr<'static>,
+        minors_start: u16,
+        this_module: &'static crate::ThisModule,
+    ) -> Self {
+        Registration {
+            name,
+            minors_start,
+            this_module,
+            inner: None,
         }
-        self.file_ops
-            .push(&file_operations::FileOperationsVtable::<T>::VTABLE);
-        self
     }
 
-    pub fn build(self, this_module: &'static crate::ThisModule) -> KernelResult<Registration> {
-        let mut dev: bindings::dev_t = 0;
-        let res = unsafe {
-            bindings::alloc_chrdev_region(
-                &mut dev,
-                self.minors.start.into(),
-                self.minors.len().try_into()?,
-                self.name.as_ptr() as *const c_types::c_char,
-            )
-        };
-        if res != 0 {
-            return Err(Error::from_kernel_errno(res));
+    pub fn register<T: file_operations::FileOperations>(self: Pin<&mut Self>) -> KernelResult<()> {
+        // 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 
+            let res = unsafe {
+                bindings::alloc_chrdev_region(
+                    &mut dev,
+                    this.minors_start.into(),
+                    N.try_into()?,
+                    this.name.as_ptr() as *const c_types::c_char,
+                )
+            };
+            if res != 0 {
+                return Err(Error::from_kernel_errno(res));
+            }
+            this.inner = Some(RegistrationInner {
+                dev: dev,
+                used: 0,
+                cdevs: [MaybeUninit::<bindings::cdev>::uninit(); N],
+                _pin: PhantomPinned,
+            });
         }
 
-        // Turn this into a boxed slice immediately because the kernel stores pointers into it, and
-        // so that data should never be moved.
-        let mut cdevs = vec![unsafe { mem::zeroed() }; self.file_ops.len()].into_boxed_slice();
-        for (i, file_op) in self.file_ops.iter().enumerate() {
-            unsafe {
-                bindings::cdev_init(&mut cdevs[i], *file_op);
-                cdevs[i].owner = this_module.0;
-                let rc = bindings::cdev_add(&mut cdevs[i], dev + i as bindings::dev_t, 1);
-                if rc != 0 {
-                    // Clean up the ones that were allocated.
-                    for j in 0..=i {
-                        bindings::cdev_del(&mut cdevs[j]);
-                    }
-                    bindings::unregister_chrdev_region(dev, self.minors.len() as _);
-                    return Err(Error::from_kernel_errno(rc));
-                }
+        let mut inner = this.inner.as_mut().unwrap();
+        if inner.used == N {
+            panic!("More devices registered than minor numbers allocated.")
+        }
+        let cdev = inner.cdevs[inner.used].as_mut_ptr();
+        // SAFETY: calling unsafe functions and manipulating MaybeUninit ptr.
+        unsafe {
+            bindings::cdev_init(cdev, &file_operations::FileOperationsVtable::<T>::VTABLE);
+            (*cdev).owner = this.this_module.0;
+            let rc = bindings::cdev_add(cdev, inner.dev + inner.used as bindings::dev_t, 1);
+            if rc != 0 {
+                return Err(Error::from_kernel_errno(rc));
             }
         }
-
-        Ok(Registration {
-            dev,
-            count: self.minors.len(),
-            cdevs,
-        })
+        inner.used += 1;
+        Ok(())
     }
 }
 
-pub struct Registration {
-    dev: bindings::dev_t,
-    count: usize,
-    cdevs: Box<[bindings::cdev]>,
-}
-
-// This is safe because Registration doesn't actually expose any methods.
-unsafe impl Sync for Registration {}
+// SAFETY: `Registration` doesn't expose any of its state across threads (it's
+// fine for multiple threads to have a shared reference to it).
+unsafe impl<const N: usize> Sync for Registration<{ N }> {}
 
-impl Drop for Registration {
+impl<const N: usize> Drop for Registration<{ N }> {
     fn drop(&mut self) {
-        unsafe {
-            for dev in self.cdevs.iter_mut() {
-                bindings::cdev_del(dev);
+        if let Some(inner) = self.inner.as_mut() {
+            // SAFETY: calling unsafe functions, `0..inner.used` of
+            // `inner.cdevs` are initialized in `Registration::register`.
+            unsafe {
+                for i in 0..inner.used {
+                    bindings::cdev_del(inner.cdevs[i].as_mut_ptr());
+                }
+                bindings::unregister_chrdev_region(inner.dev, N.try_into().unwrap());
             }
-            bindings::unregister_chrdev_region(self.dev, self.count as _);
         }
     }
 }