Skip to content

Commit

Permalink
[RFC] Rust: A fallible from_kernel_errno with Result<Error> return
Browse files Browse the repository at this point in the history
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<Error>.
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.
  • Loading branch information
foxhlchen committed Jun 4, 2021
1 parent 34658e6 commit d0ac814
Showing 1 changed file with 4 additions and 9 deletions.
13 changes: 4 additions & 9 deletions rust/kernel/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<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 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.
Expand Down

0 comments on commit d0ac814

Please sign in to comment.