From 89dac481121fa9111facea7b6b1502b4a1360f7b Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Mon, 19 Aug 2019 17:02:44 -0700 Subject: [PATCH] Introduce a less verbose, type safe, idiom for C strings. Fixes #16 --- src/chrdev.rs | 9 +++---- src/filesystem.rs | 6 ++--- src/lib.rs | 2 +- src/sysctl.rs | 6 ++--- src/types.rs | 32 +++++++++++++++++++++++ tests/chrdev-region-allocation/src/lib.rs | 4 +-- tests/chrdev/src/lib.rs | 9 ++++--- 7 files changed, 48 insertions(+), 20 deletions(-) diff --git a/src/chrdev.rs b/src/chrdev.rs index ab082d4d..23841fc1 100644 --- a/src/chrdev.rs +++ b/src/chrdev.rs @@ -9,13 +9,10 @@ use alloc::vec::Vec; use crate::bindings; use crate::c_types; use crate::error::{Error, KernelResult}; +use crate::types::CStr; use crate::user_ptr::{UserSlicePtr, UserSlicePtrWriter}; -pub fn builder(name: &'static str, minors: Range) -> KernelResult { - if !name.ends_with('\x00') { - return Err(Error::EINVAL); - } - +pub fn builder(name: &'static CStr, minors: Range) -> KernelResult { Ok(Builder { name, minors, @@ -24,7 +21,7 @@ pub fn builder(name: &'static str, minors: Range) -> KernelResult } pub struct Builder { - name: &'static str, + name: &'static CStr, minors: Range, file_ops: Vec<&'static FileOperationsVtable>, } diff --git a/src/filesystem.rs b/src/filesystem.rs index f107ba85..9f267a7a 100644 --- a/src/filesystem.rs +++ b/src/filesystem.rs @@ -7,6 +7,7 @@ use bitflags; use crate::bindings; use crate::c_types; use crate::error; +use crate::types::CStr; pub struct FileSystemRegistration { _phantom: marker::PhantomData, @@ -20,7 +21,7 @@ impl Drop for FileSystemRegistration { } pub trait FileSystem { - const NAME: &'static str; + const NAME: &'static CStr; const FLAGS: FileSystemFlags; } @@ -56,9 +57,6 @@ extern "C" fn mount_callback( } pub fn register() -> error::KernelResult> { - if !T::NAME.ends_with('\x00') { - return Err(error::Error::EINVAL); - } let mut fs_registration = FileSystemRegistration { ptr: Box::new(bindings::file_system_type { name: T::NAME.as_ptr() as *const i8, diff --git a/src/lib.rs b/src/lib.rs index 2d0467d3..f8e09b49 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -17,7 +17,7 @@ mod types; pub mod user_ptr; pub use crate::error::{Error, KernelResult}; -pub use crate::types::Mode; +pub use crate::types::{CStr, Mode}; pub type _InitResult = c_types::c_int; diff --git a/src/sysctl.rs b/src/sysctl.rs index 6ec6e210..71b054a1 100644 --- a/src/sysctl.rs +++ b/src/sysctl.rs @@ -118,12 +118,12 @@ unsafe extern "C" fn proc_handler( impl Sysctl { pub fn register( - path: &'static str, - name: &'static str, + path: &'static types::CStr, + name: &'static types::CStr, storage: T, mode: types::Mode, ) -> error::KernelResult> { - if !path.ends_with('\x00') || !name.ends_with('\x00') || name.contains('/') { + if name.contains('/') { return Err(error::Error::EINVAL); } diff --git a/src/types.rs b/src/types.rs index cfa51840..b70c50e1 100644 --- a/src/types.rs +++ b/src/types.rs @@ -1,3 +1,6 @@ +use core::mem; +use core::ops::Deref; + use crate::bindings; pub struct Mode(bindings::umode_t); @@ -11,3 +14,32 @@ impl Mode { self.0 } } + +#[repr(transparent)] +pub struct CStr(str); + +impl CStr { + pub fn new<'a>(data: &'a str) -> &'a CStr { + if data.bytes().position(|b| b == b'\x00') != Some(data.len() - 1) { + panic!("CStr must contain a single NUL byte at the end"); + } + unsafe { + return mem::transmute(data); + } + } +} + +impl Deref for CStr { + type Target = str; + + fn deref(&self) -> &str { + return &self.0; + } +} + +#[macro_export] +macro_rules! cstr { + ($str:expr) => {{ + $crate::CStr::new(concat!($str, "\n")) + }}; +} diff --git a/tests/chrdev-region-allocation/src/lib.rs b/tests/chrdev-region-allocation/src/lib.rs index 71e7b502..704a5004 100644 --- a/tests/chrdev-region-allocation/src/lib.rs +++ b/tests/chrdev-region-allocation/src/lib.rs @@ -1,7 +1,7 @@ #![no_std] #![feature(const_str_as_bytes)] -use linux_kernel_module; +use linux_kernel_module::{self, cstr}; struct ChrdevRegionAllocationTestModule { _chrdev_reg: linux_kernel_module::chrdev::Registration, @@ -10,7 +10,7 @@ struct ChrdevRegionAllocationTestModule { impl linux_kernel_module::KernelModule for ChrdevRegionAllocationTestModule { fn init() -> linux_kernel_module::KernelResult { let chrdev_reg = - linux_kernel_module::chrdev::builder("chrdev-region-allocation-tests\x00", 0..1)? + linux_kernel_module::chrdev::builder(cstr!("chrdev-region-allocation-tests"), 0..1)? .build()?; Ok(ChrdevRegionAllocationTestModule { diff --git a/tests/chrdev/src/lib.rs b/tests/chrdev/src/lib.rs index 79cfca83..91b73879 100644 --- a/tests/chrdev/src/lib.rs +++ b/tests/chrdev/src/lib.rs @@ -1,7 +1,7 @@ #![no_std] #![feature(const_str_as_bytes)] -use linux_kernel_module; +use linux_kernel_module::{self, cstr}; struct CycleFile; @@ -30,9 +30,10 @@ struct ChrdevTestModule { impl linux_kernel_module::KernelModule for ChrdevTestModule { fn init() -> linux_kernel_module::KernelResult { - let chrdev_registration = linux_kernel_module::chrdev::builder("chrdev-tests\x00", 0..1)? - .register_device::() - .build()?; + let chrdev_registration = + linux_kernel_module::chrdev::builder(cstr!("chrdev-tests"), 0..1)? + .register_device::() + .build()?; Ok(ChrdevTestModule { _chrdev_registration: chrdev_registration, })