From d0ac814b6e41e123e7f1d74e73cd91f489ffb192 Mon Sep 17 00:00:00 2001 From: Fox Chen Date: Fri, 4 Jun 2021 17:03:03 +0800 Subject: [PATCH] [RFC] Rust: A fallible from_kernel_errno with Result return Currently, from_kernel_errno is an infallible function acting as a constructor for Error. In order to achieve its type invariant, We add a check in it which will prompt a warning and return Error::EINVL when errno given is invalid. While this approach ensures type invariant, it brings great ambiguities. When Error::EINVL is returned, the caller has no way to recognize whether it is a valid errno coming from the kernel or an error issued by the check. This tricky behavior may confuse developers and introduce subtle bugs. Since Error will be used in all respects of the kernel, It's definitely not a sound solution. This RFC proposes that we make from_kernel_errno return a Result. Thus, we have an explicit, clear, and fallible version of from_kernel_errno by which callers are able to know what really happened behind the scene. And it also provides certain flexibility. We pass the power to callers, they can decide how to deal with invalid `errno` case by case. --- rust/kernel/error.rs | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs index 6ac8630c8fb203..1e7912977d746e 100644 --- a/rust/kernel/error.rs +++ b/rust/kernel/error.rs @@ -62,21 +62,16 @@ impl Error { /// Creates an [`Error`] from a kernel error code. /// - /// It is a bug to pass an out-of-range `errno`. `EINVAL` would + /// It is a bug to pass an out-of-range `errno`. Err(`EINVAL`) would /// be returned in such a case. - pub(crate) fn from_kernel_errno(errno: c_types::c_int) -> Error { + pub(crate) fn from_kernel_errno(errno: c_types::c_int) -> Result { if errno < -(bindings::MAX_ERRNO as i32) || errno >= 0 { - // TODO: make it a `WARN_ONCE` once available. - crate::pr_warn!( - "attempted to create `Error` with out of range `errno`: {}", - errno - ); - return Error::EINVAL; + return Err(Error::EINVAL); } // INVARIANT: the check above ensures the type invariant // will hold. - Error(errno) + Ok(Error(errno)) } /// Creates an [`Error`] from a kernel error code.