[PATCH v4] rust: error: modify `from_errno` to use `try_from_errno`

[PATCH v4] rust: error: modify `from_errno` to use `try_from_errno`

[Daniel Sedlak]

Modify the from_errno function to use try_from_errno to
reduce code duplication while still maintaining all existing
behavior and error handling and also reduces unsafe code.

Link: https://github.com/Rust-for-Linux/linux/issues/1125
Suggested-by:  Miguel Ojeda <ojeda@kernel.org>
Co-developed-by: Guilherme Augusto Martins da Silva <guilhermev2huehue@gmail.com>
Signed-off-by: Guilherme Augusto Martins da Silva <guilhermev2huehue@gmail.com>
Signed-off-by: Daniel Sedlak <daniel@sedlak.dev>
---
v3 -> v4:
     - Now everything should be rebased correctly.
     - Link to previous version [4]

v1 -> v3:

Currently, there are three patches for this issue. There was a race condition between
me [1] and Guilherme [2] because we both picked it up as a "good first issue" without
knowing that one of us had started working on it. We both agreed that one of us would
be the main author and the other one would be noted in "Codeveloped-by." However,
Guilherme has kept radio silence since [3]. So, I am resubmitting the patch, which
builds on top of [3]. 

Link: https://lore.kernel.org/rust-for-linux/20241104185135.18974-1-daniel@sedlak.dev/ [1]
Link: https://lore.kernel.org/rust-for-linux/20241104200014.12996-1-guilhermev2huehue@gmail.com/ [2]
Link: https://lore.kernel.org/rust-for-linux/20241105114819.14051-1-guilhermev2huehue@gmail.com/ [3]
Link: https://lore.kernel.org/rust-for-linux/20241123104857.49385-1-daniel@sedlak.dev/ [4]

 rust/kernel/error.rs | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
index 52c502432447..9b508f10c211 100644
--- a/rust/kernel/error.rs
+++ b/rust/kernel/error.rs
@@ -101,19 +101,16 @@ impl Error {
     /// It is a bug to pass an out-of-range `errno`. `EINVAL` would
     /// be returned in such a case.
     pub fn from_errno(errno: crate::ffi::c_int) -> Error {
-        if errno < -(bindings::MAX_ERRNO as i32) || errno >= 0 {
+        if let Some(error) = Self::try_from_errno(errno) {
+            error
+        } else {
             // TODO: Make it a `WARN_ONCE` once available.
             crate::pr_warn!(
                 "attempted to create `Error` with out of range `errno`: {}",
                 errno
             );
-            return code::EINVAL;
+            code::EINVAL
         }
-
-        // INVARIANT: The check above ensures the type invariant
-        // will hold.
-        // SAFETY: `errno` is checked above to be in a valid range.
-        unsafe { Error::from_errno_unchecked(errno) }
     }
 
     /// Creates an [`Error`] from a kernel error code.

---
base-commit: 40384c840ea1944d7c5a392e8975ed088ecf0b37

Daniel
-- 
2.47.1

Re: [PATCH v4] rust: error: modify `from_errno` to use `try_from_errno`

[Fiona Behrens]

On 7 Dec 2024, at 12:24, Daniel Sedlak wrote:

> Modify the from_errno function to use try_from_errno to
> reduce code duplication while still maintaining all existing
> behavior and error handling and also reduces unsafe code.
>
> Link: https://github.com/Rust-for-Linux/linux/issues/1125
> Suggested-by:  Miguel Ojeda <ojeda@kernel.org>
> Co-developed-by: Guilherme Augusto Martins da Silva <guilhermev2huehue@gmail.com>
> Signed-off-by: Guilherme Augusto Martins da Silva <guilhermev2huehue@gmail.com>
> Signed-off-by: Daniel Sedlak <daniel@sedlak.dev>

Reviewed-by: Fiona Behrens <me@kloenk.dev>

> ---
> v3 -> v4:
>      - Now everything should be rebased correctly.
>      - Link to previous version [4]
>
> v1 -> v3:
>
> Currently, there are three patches for this issue. There was a race condition between
> me [1] and Guilherme [2] because we both picked it up as a "good first issue" without
> knowing that one of us had started working on it. We both agreed that one of us would
> be the main author and the other one would be noted in "Codeveloped-by." However,
> Guilherme has kept radio silence since [3]. So, I am resubmitting the patch, which
> builds on top of [3].
>
> Link: https://lore.kernel.org/rust-for-linux/20241104185135.18974-1-daniel@sedlak.dev/ [1]
> Link: https://lore.kernel.org/rust-for-linux/20241104200014.12996-1-guilhermev2huehue@gmail.com/ [2]
> Link: https://lore.kernel.org/rust-for-linux/20241105114819.14051-1-guilhermev2huehue@gmail.com/ [3]
> Link: https://lore.kernel.org/rust-for-linux/20241123104857.49385-1-daniel@sedlak.dev/ [4]
>
>  rust/kernel/error.rs | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
> index 52c502432447..9b508f10c211 100644
> --- a/rust/kernel/error.rs
> +++ b/rust/kernel/error.rs
> @@ -101,19 +101,16 @@ impl Error {
>      /// It is a bug to pass an out-of-range `errno`. `EINVAL` would
>      /// be returned in such a case.
>      pub fn from_errno(errno: crate::ffi::c_int) -> Error {
> -        if errno < -(bindings::MAX_ERRNO as i32) || errno >= 0 {
> +        if let Some(error) = Self::try_from_errno(errno) {
> +            error
> +        } else {
>              // TODO: Make it a `WARN_ONCE` once available.
>              crate::pr_warn!(
>                  "attempted to create `Error` with out of range `errno`: {}",
>                  errno
>              );
> -            return code::EINVAL;
> +            code::EINVAL
>          }
> -
> -        // INVARIANT: The check above ensures the type invariant
> -        // will hold.
> -        // SAFETY: `errno` is checked above to be in a valid range.
> -        unsafe { Error::from_errno_unchecked(errno) }
>      }
>
>      /// Creates an [`Error`] from a kernel error code.
>
> ---
> base-commit: 40384c840ea1944d7c5a392e8975ed088ecf0b37
>
> Daniel
> -- 
> 2.47.1

Re: [PATCH v4] rust: error: modify `from_errno` to use `try_from_errno`

[Miguel Ojeda]

On Sat, Dec 7, 2024 at 12:25 PM Daniel Sedlak <daniel@sedlak.dev> wrote:
>
> Modify the from_errno function to use try_from_errno to
> reduce code duplication while still maintaining all existing
> behavior and error handling and also reduces unsafe code.
>
> Link: https://github.com/Rust-for-Linux/linux/issues/1125
> Suggested-by:  Miguel Ojeda <ojeda@kernel.org>
> Co-developed-by: Guilherme Augusto Martins da Silva <guilhermev2huehue@gmail.com>
> Signed-off-by: Guilherme Augusto Martins da Silva <guilhermev2huehue@gmail.com>
> Signed-off-by: Daniel Sedlak <daniel@sedlak.dev>

Applied to `rust-next` -- thanks everyone!

Cheers,
Miguel