Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use zero-cost method to construct Errors #335

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions rust/kernel/chrdev.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use core::pin::Pin;

use crate::bindings;
use crate::c_types;
use crate::error::{Error, Result};
use crate::error::{from_kernel_int_result, Error, Result};
use crate::file_operations;
use crate::str::CStr;

Expand Down Expand Up @@ -60,10 +60,9 @@ impl Cdev {
// - [`(*self.0).owner`] will live at least as long as the
// module, which is an implicit requirement.
let rc = unsafe { bindings::cdev_add(self.0, dev, count) };
if rc != 0 {
return Err(Error::from_kernel_errno(rc));
}
Ok(())
// SAFETY: `bindings::cdev_add()` returns zero on success,
// or a valid negative `errno` on error.
unsafe { from_kernel_int_result(rc) }
}
}

Expand Down Expand Up @@ -149,8 +148,10 @@ impl<const N: usize> Registration<{ N }> {
this.name.as_char_ptr(),
)
};
if res != 0 {
return Err(Error::from_kernel_errno(res));
// SAFETY: `bindings::alloc_chrdev_region()` returns zero on
// success, or a valid negative `errno` on error.
unsafe {
from_kernel_int_result(res)?;
}
const NONE: Option<Cdev> = None;
this.inner = Some(RegistrationInner {
Expand Down
69 changes: 69 additions & 0 deletions rust/kernel/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ impl Error {
///
/// It is a bug to pass an out-of-range `errno`. `EINVAL` would
/// be returned in such a case.
// TODO: remove `dead_code` marker once an in-kernel client is available.
#[allow(dead_code)]
pub(crate) fn from_kernel_errno(errno: c_types::c_int) -> Error {
if errno < -(bindings::MAX_ERRNO as i32) || errno >= 0 {
// TODO: make it a `WARN_ONCE` once available.
Expand Down Expand Up @@ -263,3 +265,70 @@ pub(crate) fn from_kernel_err_ptr<T>(ptr: *mut T) -> Result<*mut T> {
}
Ok(ptr)
}

/// Transform a kernel integer result to a [`Result`].
///
/// Some kernel C API functions return a result in the form of an integer:
/// zero if ok, a negative errno on error. This function converts such a
/// return value into an idiomatic [`Result`].
///
/// Use this function when the C function only returns 0 on success or
/// negative on error. If the C function returns a useful value on the
/// happy path, use [`from_kernel_int_result_uint`] instead.
///
/// # Safety
///
/// `retval` must be non-negative or a valid negative errno (i.e. `retval` must
/// be in `[-MAX_ERRNO..]`).
///
/// # Examples
///
/// ```rust,no_run
/// let ret = unsafe { bindings::misc_register(&mut this.mdev) };
/// // SAFETY: `misc_register` returns zero on success, or a valid
/// // negative errno on failure.
/// unsafe { from_kernel_int_result(ret)?; }
/// ```
pub(crate) unsafe fn from_kernel_int_result(retval: c_types::c_int) -> Result {
ojeda marked this conversation as resolved.
Show resolved Hide resolved
if retval < 0 {
// SAFETY: This condition together with the function precondition
// guarantee that `errno` is a valid negative `errno`.
return Err(unsafe { Error::from_kernel_errno_unchecked(retval) });
}
Ok(())
}

/// Transform a kernel integer result to a [`Result<c_types::c_uint>`].
///
/// Some kernel C API functions return a result in the form of an integer:
/// zero or positive if ok, a negative errno on error. This function converts
/// such a return value into an idiomatic [`Result<c_types::c_uint>`].
///
/// Use this function when the C function returns a useful, positive value
/// on success, or negative on error. If the C function only returns 0 on
/// success, use [`from_kernel_int_result`] instead.
///
/// # Safety
///
/// `retval` must be non-negative or a valid negative errno (i.e. `retval` must
/// be in `[-MAX_ERRNO..]`).
///
/// # Examples
///
/// ```rust,no_run
/// let fd = unsafe { bindings::get_unused_fd_flags(flags) };
/// // SAFETY: `bindings::get_unused_fd_flags()` returns a non-negative
/// // `fd` on success, or a valid negative `errno` on error.
/// let fd = unsafe { from_kernel_int_result_uint(fd)? };
/// ```
pub(crate) unsafe fn from_kernel_int_result_uint(
retval: c_types::c_int,
) -> Result<c_types::c_uint> {
if retval < 0 {
// SAFETY: This condition together with the function precondition
// guarantee that `errno` is a valid negative `errno`.
return Err(unsafe { Error::from_kernel_errno_unchecked(retval) });
}
// CAST: a non-negative `c_types::c_int` always fits in a `c_types::c_uint`.
Ok(retval as c_types::c_uint)
}
12 changes: 8 additions & 4 deletions rust/kernel/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@
//! C headers: [`include/linux/fs.h`](../../../../include/linux/fs.h) and
//! [`include/linux/file.h`](../../../../include/linux/file.h)

use crate::{bindings, error::Error, Result};
use crate::{
bindings,
error::{from_kernel_int_result_uint, Error},
Result,
};
use core::{mem::ManuallyDrop, ops::Deref};

/// Wraps the kernel's `struct file`.
Expand Down Expand Up @@ -96,9 +100,9 @@ impl FileDescriptorReservation {
/// Creates a new file descriptor reservation.
pub fn new(flags: u32) -> Result<Self> {
let fd = unsafe { bindings::get_unused_fd_flags(flags) };
if fd < 0 {
return Err(Error::from_kernel_errno(fd));
}
// SAFETY: `bindings::get_unused_fd_flags()` returns a non-negative
// `fd` on success, or a valid negative `errno` on error.
let fd = unsafe { from_kernel_int_result_uint(fd)? };
Ok(Self { fd: fd as _ })
}

Expand Down
8 changes: 5 additions & 3 deletions rust/kernel/miscdev.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
//! Reference: <https://www.kernel.org/doc/html/latest/driver-api/misc_devices.html>

use crate::bindings;
use crate::error::{Error, Result};
use crate::error::{from_kernel_int_result, Error, Result};
use crate::file_operations::{FileOpenAdapter, FileOpener, FileOperationsVtable};
use crate::str::CStr;
use alloc::boxed::Box;
Expand Down Expand Up @@ -73,8 +73,10 @@ impl<T: Sync> Registration<T> {
this.mdev.minor = minor.unwrap_or(bindings::MISC_DYNAMIC_MINOR as i32);

let ret = unsafe { bindings::misc_register(&mut this.mdev) };
if ret < 0 {
return Err(Error::from_kernel_errno(ret));
// SAFETY: `bindings::alloc_chrdev_region()` returns zero on
// success, or a valid negative `errno` on error.
unsafe {
from_kernel_int_result(ret)?;
}
this.registered = true;
Ok(())
Expand Down
12 changes: 5 additions & 7 deletions rust/kernel/pages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
//! TODO: This module is a work in progress.

use crate::{
bindings, c_types, io_buffer::IoBufferReader, user_ptr::UserSlicePtrReader, Error, Result,
PAGE_SIZE,
bindings, c_types, error::from_kernel_int_result, io_buffer::IoBufferReader,
user_ptr::UserSlicePtrReader, Error, Result, PAGE_SIZE,
};
use core::{marker::PhantomData, ptr};

Expand Down Expand Up @@ -65,11 +65,9 @@ impl<const ORDER: u32> Pages<ORDER> {
// SAFETY: We check above that the allocation is of order 0. The range of `address` is
// already checked by `vm_insert_page`.
let ret = unsafe { bindings::vm_insert_page(vma, address as _, self.pages) };
if ret != 0 {
Err(Error::from_kernel_errno(ret))
} else {
Ok(())
}
// SAFETY: `bindings::vm_insert_page()` returns zero on success,
// or a valid negative `errno` on error.
unsafe { from_kernel_int_result(ret) }
}

/// Copies data from the given [`UserSlicePtrReader`] into the pages.
Expand Down
8 changes: 5 additions & 3 deletions rust/kernel/platdev.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

use crate::{
bindings, c_types,
error::{Error, Result},
error::{from_kernel_int_result, Error, Result},
from_kernel_result,
of::OfMatchTable,
str::CStr,
Expand Down Expand Up @@ -83,8 +83,10 @@ impl Registration {
// `bindings::platform_driver_unregister()`, or
// - null.
let ret = unsafe { bindings::__platform_driver_register(&mut this.pdrv, module.0) };
if ret < 0 {
return Err(Error::from_kernel_errno(ret));
// SAFETY: `bindings::__platform_driver_register()` returns zero on
// success, or a valid negative `errno` on error.
unsafe {
from_kernel_int_result(ret)?;
}
this.registered = true;
Ok(())
Expand Down
11 changes: 8 additions & 3 deletions rust/kernel/random.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,21 @@

use core::convert::TryInto;

use crate::{bindings, c_types, error};
use crate::{
bindings, c_types,
error::{self, from_kernel_int_result},
};

/// Fills a byte slice with random bytes generated from the kernel's CSPRNG.
///
/// Ensures that the CSPRNG has been seeded before generating any random bytes,
/// and will block until it is ready.
pub fn getrandom(dest: &mut [u8]) -> error::Result {
let res = unsafe { bindings::wait_for_random_bytes() };
if res != 0 {
return Err(error::Error::from_kernel_errno(res));
// SAFETY: `bindings::wait_for_random_bytes()` returns zero on success,
// or a valid negative `errno` on error.
unsafe {
from_kernel_int_result(res)?;
}

unsafe {
Expand Down