Skip to content

rust/kernel: move from_kernel_result! macro to error.rs #266

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

Merged
merged 3 commits into from
May 19, 2021
Merged
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
48 changes: 48 additions & 0 deletions rust/kernel/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use crate::{bindings, c_types};
use alloc::{alloc::AllocError, collections::TryReserveError};
use core::convert::From;
use core::{num::TryFromIntError, str::Utf8Error};

/// Generic integer kernel error.
Expand Down Expand Up @@ -104,3 +105,50 @@ impl From<AllocError> for Error {
Error::ENOMEM
}
}

// # Invariant: `-bindings::MAX_ERRNO` fits in an `i16`.
crate::static_assert!(bindings::MAX_ERRNO <= -(i16::MIN as i32) as u32);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have the nagging feeling that this method is too contrived. Is there an idiomatic way to verify whether a value fits in a type? I tried Try or TryFrom but that doesn't work, because those types can't run inside a static_assert!.

Copy link
Member

@ojeda ojeda May 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have taken a look at this -- it is indeed a bit painful. After having tried several approaches, I am preparing a PR with a fun one.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am grabbing the popcorn !! :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See if you like #269 :)

It would allow you to do:

static_assert!((-(bindings::MAX_ERRNO as i128)) fits in i16);

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it a lot !! Will definitely rebase to that after it gets merged !! 💯


#[doc(hidden)]
pub fn from_kernel_result_helper<T>(r: Result<T>) -> T
where
T: From<i16>,
{
match r {
Ok(v) => v,
// NO-OVERFLOW: negative `errno`s are no smaller than `-bindings::MAX_ERRNO`,
// `-bindings::MAX_ERRNO` fits in an `i16` as per invariant above,
// therefore a negative `errno` always fits in an `i16` and will not overflow.
Err(e) => T::from(e.to_kernel_errno() as i16),
}
}

/// Transforms a [`crate::error::Result<T>`] to a kernel C integer result.
///
/// This is useful when calling Rust functions that return [`crate::error::Result<T>`]
/// from inside `extern "C"` functions that need to return an integer
/// error result.
///
/// `T` should be convertible to an `i16` via `From<i16>`.
///
/// # Examples
///
/// ```rust,no_run
/// unsafe extern "C" fn probe_callback(
/// pdev: *mut bindings::platform_device,
/// ) -> c_types::c_int {
/// from_kernel_result! {
/// let ptr = devm_alloc(pdev)?;
/// rust_helper_platform_set_drvdata(pdev, ptr);
/// Ok(0)
/// }
/// }
/// ```
#[macro_export]
macro_rules! from_kernel_result {
($($tt:tt)*) => {{
$crate::error::from_kernel_result_helper((|| {
$($tt)*
})())
}};
}
20 changes: 1 addition & 19 deletions rust/kernel/file_operations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use crate::{
bindings, c_types,
error::{Error, Result},
file::File,
from_kernel_result,
io_buffer::{IoBufferReader, IoBufferWriter},
iov_iter::IovIter,
sync::CondVar,
Expand Down Expand Up @@ -78,25 +79,6 @@ pub enum SeekFrom {
Current(i64),
}

fn from_kernel_result<T>(r: Result<T>) -> T
where
T: TryFrom<c_types::c_int>,
T::Error: core::fmt::Debug,
{
match r {
Ok(v) => v,
Err(e) => T::try_from(e.to_kernel_errno()).unwrap(),
}
}

macro_rules! from_kernel_result {
($($tt:tt)*) => {{
from_kernel_result((|| {
$($tt)*
})())
}};
}

unsafe extern "C" fn open_callback<A: FileOpenAdapter, T: FileOpener<A::Arg>>(
inode: *mut bindings::inode,
file: *mut bindings::file,
Expand Down