Messages in this thread Patch in this message | | | Date | Sat, 25 Feb 2023 18:22:11 -0800 | From | Boqun Feng <> | Subject | Re: [PATCH 5/5] rust: error: Add from_kernel_result!() macro |
| |
On Sat, Feb 25, 2023 at 10:23:40PM +0000, Gary Guo wrote: > On Fri, 24 Feb 2023 15:56:05 -0800 > Boqun Feng <boqun.feng@gmail.com> wrote: > > > On Fri, Feb 24, 2023 at 05:50:23PM +0900, Asahi Lina wrote: > > > From: Wedson Almeida Filho <wedsonaf@gmail.com> > > > > > > Add a helper macro to easily return C result codes from a Rust function > > > that calls functions which return a Result<T>. > > > > > > Lina: Imported from rust-for-linux/rust, originally developed by Wedson > > > as part of file_operations.rs. Added the allow() flags since there is no > > > user in the kernel crate yet and fixed a typo in a comment. > > > > > > Co-developed-by: Fox Chen <foxhlchen@gmail.com> > > > Signed-off-by: Fox Chen <foxhlchen@gmail.com> > > > Co-developed-by: Miguel Ojeda <ojeda@kernel.org> > > > Signed-off-by: Miguel Ojeda <ojeda@kernel.org> > > > Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com> > > > Signed-off-by: Asahi Lina <lina@asahilina.net> > > > --- > > > rust/kernel/error.rs | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > > > 1 file changed, 52 insertions(+) > > > > > > diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs > > > index cf3d089477d2..8a9222595cd1 100644 > > > --- a/rust/kernel/error.rs > > > +++ b/rust/kernel/error.rs > > > @@ -226,3 +226,55 @@ pub(crate) fn from_kernel_err_ptr<T>(ptr: *mut T) -> Result<*mut T> { > > > } > > > Ok(ptr) > > > } > > > + > > > +// TODO: Remove `dead_code` marker once an in-kernel client is available. > > > +#[allow(dead_code)] > > > +pub(crate) 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 from an `i16` via `From<i16>`. > > > +/// > > > +/// # Examples > > > +/// > > > +/// ```ignore > > > +/// # use kernel::from_kernel_result; > > > +/// # use kernel::bindings; > > > +/// unsafe extern "C" fn probe_callback( > > > +/// pdev: *mut bindings::platform_device, > > > +/// ) -> core::ffi::c_int { > > > +/// from_kernel_result! { > > > +/// let ptr = devm_alloc(pdev)?; > > > +/// bindings::platform_set_drvdata(pdev, ptr); > > > +/// Ok(0) > > > +/// } > > > +/// } > > > +/// ``` > > > +// TODO: Remove `unused_macros` marker once an in-kernel client is available. > > > +#[allow(unused_macros)] > > > +macro_rules! from_kernel_result { > > > > This actually doesn't need to be a macro, right? The following function > > version: > > > > pub fn from_kernel_result<T, F>(f: F) -> T > > where > > T: From<i16>, > > F: FnOnce() -> Result<T>; > > > > is not bad, the above case then can be written as: > > > > unsafe extern "C" fn probe_callback( > > pdev: *mut bindings::platform_device, > > ) -> core::ffi::c_int { > > from_kernel_result(|| { > > let ptr = devm_alloc(pdev)?; > > bindings::platform_set_drvdata(pdev, ptr); > > Ok(0) > > }) > > } > > > > less magical, but the control flow is more clear. > > > > Thoughts? > > I don't think even the closure is necessary? Why not just > > pub fn from_kernel_result<T: From<i16>>(r: Result<T>) -> T > > and > > unsafe extern "C" fn probe_callback( > pdev: *mut bindings::platform_device, > ) -> core::ffi::c_int { > from_kernel_result({ > let ptr = devm_alloc(pdev)?;
Hmm.. looks like the `?` only "propagating them (the errors) to the calling *function*":
https://doc.rust-lang.org/reference/expressions/operator-expr.html#the-question-mark-operator
, so this doesn't work as we expect: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=64c9039e1da2c436f9f4f5ea46e97e90
Hope I didn't miss something subtle here..
Regards, Boqun
> bindings::platform_set_drvdata(pdev, ptr); > Ok(0) > }) > } > > ? > > Best, > Gary
| |