[PATCH 2/5] rust: error: Add Error::from_kernel_errno()

Asahi Lina posted 5 patches 2 years, 6 months ago
There is a newer version of this series
[PATCH 2/5] rust: error: Add Error::from_kernel_errno()
Posted by Asahi Lina 2 years, 6 months ago
From: Miguel Ojeda <ojeda@kernel.org>

Add a function to create `Error` values out of a kernel error return,
which safely upholds the invariant that the error code is well-formed
(negative and greater than -MAX_ERRNO). If a malformed code is passed
in, it will be converted to EINVAL.

Lina: Imported from rust-for-linux/rust as authored by Miguel and Fox
with refactoring from Wedson.

Co-developed-by: Fox Chen <foxhlchen@gmail.com>
Signed-off-by: Fox Chen <foxhlchen@gmail.com>
Co-developed-by: Wedson Almeida Filho <wedsonaf@gmail.com>
Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Signed-off-by: Asahi Lina <lina@asahilina.net>
---
 rust/kernel/error.rs | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
index 8611758e27f4..3b439fdb405c 100644
--- a/rust/kernel/error.rs
+++ b/rust/kernel/error.rs
@@ -72,6 +72,25 @@ pub mod code {
 pub struct Error(core::ffi::c_int);
 
 impl Error {
+    /// Creates an [`Error`] from a kernel error code.
+    ///
+    /// It is a bug to pass an out-of-range `errno`. `EINVAL` would
+    /// be returned in such a case.
+    pub(crate) fn from_kernel_errno(errno: core::ffi::c_int) -> Error {
+        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 code::EINVAL;
+        }
+
+        // INVARIANT: The check above ensures the type invariant
+        // will hold.
+        Error(errno)
+    }
+
     /// Returns the kernel error code.
     pub fn to_kernel_errno(self) -> core::ffi::c_int {
         self.0

-- 
2.35.1
Re: [PATCH 2/5] rust: error: Add Error::from_kernel_errno()
Posted by Andreas Hindborg 2 years, 6 months ago
Asahi Lina <lina@asahilina.net> writes:

> From: Miguel Ojeda <ojeda@kernel.org>
>
> Add a function to create `Error` values out of a kernel error return,
> which safely upholds the invariant that the error code is well-formed
> (negative and greater than -MAX_ERRNO). If a malformed code is passed
> in, it will be converted to EINVAL.
>
> Lina: Imported from rust-for-linux/rust as authored by Miguel and Fox
> with refactoring from Wedson.
>
> Co-developed-by: Fox Chen <foxhlchen@gmail.com>
> Signed-off-by: Fox Chen <foxhlchen@gmail.com>
> Co-developed-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> Signed-off-by: Asahi Lina <lina@asahilina.net>
> ---

Reviewed-by: Andreas Hindborg <a.hindborg@samsung.com>

>  rust/kernel/error.rs | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>
> diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
> index 8611758e27f4..3b439fdb405c 100644
> --- a/rust/kernel/error.rs
> +++ b/rust/kernel/error.rs
> @@ -72,6 +72,25 @@ pub mod code {
>  pub struct Error(core::ffi::c_int);
>  
>  impl Error {
> +    /// Creates an [`Error`] from a kernel error code.
> +    ///
> +    /// It is a bug to pass an out-of-range `errno`. `EINVAL` would
> +    /// be returned in such a case.
> +    pub(crate) fn from_kernel_errno(errno: core::ffi::c_int) -> Error {
> +        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 code::EINVAL;
> +        }
> +
> +        // INVARIANT: The check above ensures the type invariant
> +        // will hold.
> +        Error(errno)
> +    }
> +
>      /// Returns the kernel error code.
>      pub fn to_kernel_errno(self) -> core::ffi::c_int {
>          self.0
Re: [PATCH 2/5] rust: error: Add Error::from_kernel_errno()
Posted by Gary Guo 2 years, 6 months ago
On Fri, 24 Feb 2023 17:50:20 +0900
Asahi Lina <lina@asahilina.net> wrote:

> From: Miguel Ojeda <ojeda@kernel.org>
> 
> Add a function to create `Error` values out of a kernel error return,
> which safely upholds the invariant that the error code is well-formed
> (negative and greater than -MAX_ERRNO). If a malformed code is passed
> in, it will be converted to EINVAL.
> 
> Lina: Imported from rust-for-linux/rust as authored by Miguel and Fox
> with refactoring from Wedson.
> 
> Co-developed-by: Fox Chen <foxhlchen@gmail.com>
> Signed-off-by: Fox Chen <foxhlchen@gmail.com>
> Co-developed-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> Signed-off-by: Asahi Lina <lina@asahilina.net>
> ---
>  rust/kernel/error.rs | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
> index 8611758e27f4..3b439fdb405c 100644
> --- a/rust/kernel/error.rs
> +++ b/rust/kernel/error.rs
> @@ -72,6 +72,25 @@ pub mod code {
>  pub struct Error(core::ffi::c_int);
>  
>  impl Error {
> +    /// Creates an [`Error`] from a kernel error code.
> +    ///
> +    /// It is a bug to pass an out-of-range `errno`. `EINVAL` would
> +    /// be returned in such a case.
> +    pub(crate) fn from_kernel_errno(errno: core::ffi::c_int) -> Error {

Maybe just `from_errno`? I don't know why `kernel` would need to be
emphasised here.

Best,
Gary
Re: [PATCH 2/5] rust: error: Add Error::from_kernel_errno()
Posted by Miguel Ojeda 2 years, 6 months ago
On Sat, Feb 25, 2023 at 11:19 PM Gary Guo <gary@garyguo.net> wrote:
>
> Maybe just `from_errno`? I don't know why `kernel` would need to be
> emphasised here.

Yeah, that sounds good to me. It is clear and we don't use "errno"
elsewhere. This identifier came from the original import, so before we
started to think about naming policies.

Though perhaps we can clean it up in a patch later, since we should
change `to_kernel_errno` below too at the same time. Or if you want to
send a quick patch for that one, I can put it in first.

Cheers,
Miguel