From c52904bb14ffca2be909de3ea683e8ba3b97c0e2 Mon Sep 17 00:00:00 2001
From: Alex Gaynor <alex.gaynor@gmail.com>
Date: Sat, 6 Jul 2019 22:15:06 -0400
Subject: [PATCH] Refactor chrdev to use a builder pattern

---
 src/chrdev.rs                             | 45 ++++++++++++++---------
 src/error.rs                              |  8 ++++
 tests/chrdev-region-allocation/src/lib.rs | 11 +++---
 3 files changed, 41 insertions(+), 23 deletions(-)

diff --git a/src/chrdev.rs b/src/chrdev.rs
index 1179460e..ac820d34 100644
--- a/src/chrdev.rs
+++ b/src/chrdev.rs
@@ -1,44 +1,53 @@
+use core::convert::TryInto;
 use core::ops::Range;
 
 use crate::bindings;
 use crate::c_types;
 use crate::error;
 
-pub struct DeviceNumberRegion {
-    dev: bindings::dev_t,
-    count: usize,
+pub fn builder(name: &'static str, minors: Range<u16>) -> error::KernelResult<Builder> {
+    if !name.ends_with('\x00') {
+        return Err(error::Error::EINVAL);
+    }
+
+    return Ok(Builder { name, minors });
 }
 
-impl DeviceNumberRegion {
-    pub fn allocate(
-        minors: Range<usize>,
-        name: &'static str,
-    ) -> error::KernelResult<DeviceNumberRegion> {
-        if !name.ends_with('\x00') {
-            return Err(error::Error::EINVAL);
-        }
+pub struct Builder {
+    name: &'static str,
+    minors: Range<u16>,
+}
 
-        let count = minors.end - minors.start;
+impl Builder {
+    pub fn build(self) -> error::KernelResult<Registration> {
         let mut dev: bindings::dev_t = 0;
         let res = unsafe {
             bindings::alloc_chrdev_region(
                 &mut dev,
-                minors.start as bindings::dev_t,
-                count as bindings::dev_t,
-                name.as_ptr() as *const c_types::c_char,
+                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::Error::from_kernel_errno(res));
         }
-        return Ok(DeviceNumberRegion { dev, count });
+        return Ok(Registration {
+            dev,
+            count: self.minors.len(),
+        });
     }
 }
 
-impl Drop for DeviceNumberRegion {
+pub struct Registration {
+    dev: bindings::dev_t,
+    count: usize,
+}
+
+impl Drop for Registration {
     fn drop(&mut self) {
         unsafe {
-            bindings::unregister_chrdev_region(self.dev, self.count as bindings::dev_t);
+            bindings::unregister_chrdev_region(self.dev, self.count as _);
         }
     }
 }
diff --git a/src/error.rs b/src/error.rs
index f367ee25..debb5463 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -1,3 +1,5 @@
+use core::num::TryFromIntError;
+
 use crate::bindings;
 use crate::c_types;
 
@@ -17,4 +19,10 @@ impl Error {
     }
 }
 
+impl From<TryFromIntError> for Error {
+    fn from(_: TryFromIntError) -> Error {
+        return Error::EINVAL;
+    }
+}
+
 pub type KernelResult<T> = Result<T, Error>;
diff --git a/tests/chrdev-region-allocation/src/lib.rs b/tests/chrdev-region-allocation/src/lib.rs
index 8808ea0b..71e7b502 100644
--- a/tests/chrdev-region-allocation/src/lib.rs
+++ b/tests/chrdev-region-allocation/src/lib.rs
@@ -4,16 +4,17 @@
 use linux_kernel_module;
 
 struct ChrdevRegionAllocationTestModule {
-    _dev: linux_kernel_module::chrdev::DeviceNumberRegion,
+    _chrdev_reg: linux_kernel_module::chrdev::Registration,
 }
 
 impl linux_kernel_module::KernelModule for ChrdevRegionAllocationTestModule {
     fn init() -> linux_kernel_module::KernelResult<Self> {
+        let chrdev_reg =
+            linux_kernel_module::chrdev::builder("chrdev-region-allocation-tests\x00", 0..1)?
+                .build()?;
+
         Ok(ChrdevRegionAllocationTestModule {
-            _dev: linux_kernel_module::chrdev::DeviceNumberRegion::allocate(
-                0..1,
-                "chrdev-region-allocation-tests\x00",
-            )?,
+            _chrdev_reg: chrdev_reg,
         })
     }
 }