Hi Paolo,
> It is a common convention in QEMU to return a positive value in case of
> success, and a negated errno value in case of error. Unfortunately,
> using errno portably in Rust is a bit complicated; on Unix the errno
> values are supported natively by io::Error, but on Windows they are not;
> so, use the libc crate.
I'm a bit confused. The doc of error.h just said the negative value for
failure:
• integer-valued functions return non-negative / negative.
Why do we need to using libc's -errno for Windows as well?
Converting `io::Error::last_os_error().raw_os_error().unwrap()` to a
negative value seems compatible with Windows, except it returns Windows
error codes.
> This is a set of utility functions that are used by both chardev and
> block layer bindings.
...
> +// On Unix, from_raw_os_error takes an errno value and OS errors
> +// are printed using strerror. On Windows however it takes a
> +// GetLastError() value; therefore we need to convert errno values
> +// into io::Error by hand. This is the same mapping that the
> +// standard library uses to retrieve the kind of OS errors
> +// (`std::sys::pal::unix::decode_error_kind`).
> +impl From<Errno> for ErrorKind {
> + fn from(value: Errno) -> ErrorKind {
What about `use ErrorKind::*;` to oimt the following "ErrorKind::"
prefix?
> + let Errno(errno) = value;
> + match i32::from(errno) {
Maybe `match i32::from(errno.0)` ?
> + libc::EPERM | libc::EACCES => ErrorKind::PermissionDenied,
> + libc::ENOENT => ErrorKind::NotFound,
> + libc::EINTR => ErrorKind::Interrupted,
> + x if x == libc::EAGAIN || x == libc::EWOULDBLOCK =>
> ErrorKind::WouldBlock,
> + libc::ENOMEM => ErrorKind::OutOfMemory,
> + libc::EEXIST => ErrorKind::AlreadyExists,
> + libc::EINVAL => ErrorKind::InvalidInput,
> + libc::EPIPE => ErrorKind::BrokenPipe,
> + libc::EADDRINUSE => ErrorKind::AddrInUse,
> + libc::EADDRNOTAVAIL => ErrorKind::AddrNotAvailable,
> + libc::ECONNABORTED => ErrorKind::ConnectionAborted,
> + libc::ECONNREFUSED => ErrorKind::ConnectionRefused,
> + libc::ECONNRESET => ErrorKind::ConnectionReset,
> + libc::ENOTCONN => ErrorKind::NotConnected,
> + libc::ENOTSUP => ErrorKind::Unsupported,
> + libc::ETIMEDOUT => ErrorKind::TimedOut,
> + _ => ErrorKind::Other,
Are these errno cases specifically selected? It seems to have fewer than
`decode_error_kind` lists. Why not support all the cases `decode_error_kind`
mentions? Or do we need to try to cover as many errno cases as possible
from rust/kernel/error.rs?
> + }
> + }
> +}
> +
> +// This is used on Windows for all io::Errors, but also on Unix if the
> +// io::Error does not have a raw OS error. This is the reversed
> +// mapping of the above.
Maybe:
This is the "almost" reversed (except the default case) mapping
?
> +impl From<io::ErrorKind> for Errno {
> + fn from(value: io::ErrorKind) -> Errno {
`use ErrorKind::*;` could save some words, too.
> + let errno = match value {
> + // can be both EPERM or EACCES :( pick one
> + ErrorKind::PermissionDenied => libc::EPERM,
> + ErrorKind::NotFound => libc::ENOENT,
> + ErrorKind::Interrupted => libc::EINTR,
> + ErrorKind::WouldBlock => libc::EAGAIN,
> + ErrorKind::OutOfMemory => libc::ENOMEM,
> + ErrorKind::AlreadyExists => libc::EEXIST,
> + ErrorKind::InvalidInput => libc::EINVAL,
> + ErrorKind::BrokenPipe => libc::EPIPE,
> + ErrorKind::AddrInUse => libc::EADDRINUSE,
> + ErrorKind::AddrNotAvailable => libc::EADDRNOTAVAIL,
> + ErrorKind::ConnectionAborted => libc::ECONNABORTED,
> + ErrorKind::ConnectionRefused => libc::ECONNREFUSED,
> + ErrorKind::ConnectionReset => libc::ECONNRESET,
> + ErrorKind::NotConnected => libc::ENOTCONN,
> + ErrorKind::Unsupported => libc::ENOTSUP,
> + ErrorKind::TimedOut => libc::ETIMEDOUT,
> + _ => libc::EIO,
> + };
> + Errno(errno as u16)
> + }
> +}
> +
> +impl From<Errno> for io::Error {
> + #[cfg(unix)]
> + fn from(value: Errno) -> io::Error {
> + let Errno(errno) = value;
> + io::Error::from_raw_os_error(errno.into())
Maybe `io::Error::from_raw_os_error(value.0.into())`?
> + }
> +
> + #[cfg(windows)]
> + fn from(value: Errno) -> io::Error {
> + let error_kind: ErrorKind = value.into();
> + error_kind.into()
Even this works:
fn from(value: Errno) -> io::Error {
- let error_kind: ErrorKind = value.into();
- error_kind.into()
+ value.into()
However, it's less readability, so I still prefer your current codes.
:-)
Thanks,
Zhao
