Skip to content

Commit 865c748

Browse files
bors[bot]asomers
andauthored
Merge #1446
1446: Overhaul Nix's error types r=asomers a=asomers For many of Nix's consumers it be convenient to easily convert a Nix error into a std::io::Error. That's currently not possible because of the InvalidPath, InvalidUtf8, and UnsupportedOperation types that have no equivalent in std::io::Error. However, very few of Nix's public APIs actually return those unusual errors. So a more useful API would be for Nix's standard error type to implement Into<std::io::Error>, and the few functions that must return unusual errors like InvalidUtf8 should use bespoke error types. This commit prototypes that approach by modifying just one function, for now, to use the new error type. Co-authored-by: Alan Somers <asomers@gmail.com>
2 parents a2165e6 + 714d6e9 commit 865c748

40 files changed

+257
-256
lines changed

CHANGELOG.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,16 @@ This project adheres to [Semantic Versioning](https://semver.org/).
1414
(#[1457](https://github.com/nix-rust/nix/pull/1457))
1515

1616
### Changed
17+
- `ptsname_r` now returns a lossily-converted string in the event of bad UTF,
18+
just like `ptsname`.
19+
([#1446](https://github.com/nix-rust/nix/pull/1446))
20+
- Nix's error type is now a simple wrapper around the platform's Errno. This
21+
means it is now `Into<std::io::Error>`. It's also `Clone`, `Copy`, `Eq`, and
22+
has a small fixed size. It also requires less typing. For example, the old
23+
enum variant `nix::Error::Sys(nix::errno::Errno::EINVAL)` is now simply
24+
`nix::Error::EINVAL`.
25+
([#1446](https://github.com/nix-rust/nix/pull/1446))
26+
1727
### Fixed
1828
### Removed
1929

src/dir.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ impl AsRawFd for Dir {
8484
impl Drop for Dir {
8585
fn drop(&mut self) {
8686
let e = Errno::result(unsafe { libc::closedir(self.0.as_ptr()) });
87-
if !std::thread::panicking() && e == Err(Error::Sys(Errno::EBADF)) {
87+
if !std::thread::panicking() && e == Err(Error::from(Errno::EBADF)) {
8888
panic!("Closing an invalid file descriptor!");
8989
};
9090
}

src/env.rs

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,24 @@
11
use cfg_if::cfg_if;
2-
use crate::{Error, Result};
2+
use std::fmt;
3+
4+
/// Indicates that [`clearenv`] failed for some unknown reason
5+
#[derive(Clone, Copy, Debug)]
6+
pub struct ClearEnvError;
7+
8+
impl fmt::Display for ClearEnvError {
9+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
10+
write!(f, "clearenv failed")
11+
}
12+
}
13+
14+
impl std::error::Error for ClearEnvError {}
315

416
/// Clear the environment of all name-value pairs.
517
///
618
/// On platforms where libc provides `clearenv()`, it will be used. libc's
719
/// `clearenv()` is documented to return an error code but not set errno; if the
820
/// return value indicates a failure, this function will return
9-
/// `Error::UnsupportedOperation`.
21+
/// [`ClearEnvError`].
1022
///
1123
/// On platforms where libc does not provide `clearenv()`, a fallback
1224
/// implementation will be used that iterates over all environment variables and
@@ -25,7 +37,7 @@ use crate::{Error, Result};
2537
/// `environ` is currently held. The latter is not an issue if the only other
2638
/// environment access in the program is via `std::env`, but the requirement on
2739
/// thread safety must still be upheld.
28-
pub unsafe fn clearenv() -> Result<()> {
40+
pub unsafe fn clearenv() -> std::result::Result<(), ClearEnvError> {
2941
let ret;
3042
cfg_if! {
3143
if #[cfg(any(target_os = "fuchsia",
@@ -48,6 +60,6 @@ pub unsafe fn clearenv() -> Result<()> {
4860
if ret == 0 {
4961
Ok(())
5062
} else {
51-
Err(Error::UnsupportedOperation)
63+
Err(ClearEnvError)
5264
}
5365
}

src/errno.rs

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use cfg_if::cfg_if;
22
use libc::{c_int, c_void};
3+
use std::convert::TryFrom;
34
use std::{fmt, io, error};
45
use crate::{Error, Result};
56

@@ -48,6 +49,42 @@ pub fn errno() -> i32 {
4849
}
4950

5051
impl Errno {
52+
/// Convert this `Error` to an [`Errno`](enum.Errno.html).
53+
///
54+
/// # Example
55+
///
56+
/// ```
57+
/// # use nix::Error;
58+
/// # use nix::errno::Errno;
59+
/// let e = Error::from(Errno::EPERM);
60+
/// assert_eq!(Some(Errno::EPERM), e.as_errno());
61+
/// ```
62+
#[deprecated(
63+
since = "0.22.0",
64+
note = "It's a no-op now; just delete it."
65+
)]
66+
pub fn as_errno(self) -> Option<Self> {
67+
Some(self)
68+
}
69+
70+
/// Create a nix Error from a given errno
71+
#[deprecated(
72+
since = "0.22.0",
73+
note = "It's a no-op now; just delete it."
74+
)]
75+
pub fn from_errno(errno: Errno) -> Error {
76+
Error::from(errno)
77+
}
78+
79+
/// Create a new invalid argument error (`EINVAL`)
80+
#[deprecated(
81+
since = "0.22.0",
82+
note = "Use Errno::EINVAL instead"
83+
)]
84+
pub fn invalid_argument() -> Error {
85+
Errno::EINVAL
86+
}
87+
5188
pub fn last() -> Self {
5289
last()
5390
}
@@ -68,11 +105,26 @@ impl Errno {
68105
/// should not be used when `-1` is not the errno sentinel value.
69106
pub fn result<S: ErrnoSentinel + PartialEq<S>>(value: S) -> Result<S> {
70107
if value == S::sentinel() {
71-
Err(Error::Sys(Self::last()))
108+
Err(Self::last())
72109
} else {
73110
Ok(value)
74111
}
75112
}
113+
114+
/// Backwards compatibility hack for Nix <= 0.21.0 users
115+
///
116+
/// In older versions of Nix, `Error::Sys` was an enum variant. Now it's a
117+
/// function, which is compatible with most of the former use cases of the
118+
/// enum variant. But you should use `Error(Errno::...)` instead.
119+
#[deprecated(
120+
since = "0.22.0",
121+
note = "Use Errno::... instead"
122+
)]
123+
#[allow(non_snake_case)]
124+
#[inline]
125+
pub fn Sys(errno: Errno) -> Error {
126+
errno
127+
}
76128
}
77129

78130
/// The sentinel value indicates that a function failed and more detailed
@@ -115,6 +167,16 @@ impl From<Errno> for io::Error {
115167
}
116168
}
117169

170+
impl TryFrom<io::Error> for Errno {
171+
type Error = io::Error;
172+
173+
fn try_from(ioerror: io::Error) -> std::result::Result<Self, io::Error> {
174+
ioerror.raw_os_error()
175+
.map(Errno::from_i32)
176+
.ok_or(ioerror)
177+
}
178+
}
179+
118180
fn last() -> Errno {
119181
Errno::from_i32(errno())
120182
}

src/fcntl.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ fn inner_readlink<P: ?Sized + NixPath>(dirfd: Option<RawFd>, path: &P) -> Result
288288
Some(next_size) => try_size = next_size,
289289
// It's absurd that this would happen, but handle it sanely
290290
// anyway.
291-
None => break Err(super::Error::Sys(Errno::ENAMETOOLONG)),
291+
None => break Err(super::Error::from(Errno::ENAMETOOLONG)),
292292
}
293293
}
294294
}
@@ -646,6 +646,6 @@ pub fn posix_fallocate(fd: RawFd, offset: libc::off_t, len: libc::off_t) -> Resu
646646
match Errno::result(res) {
647647
Err(err) => Err(err),
648648
Ok(0) => Ok(()),
649-
Ok(errno) => Err(crate::Error::Sys(Errno::from_i32(errno))),
649+
Ok(errno) => Err(crate::Error::from(Errno::from_i32(errno))),
650650
}
651651
}

src/lib.rs

Lines changed: 15 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -79,91 +79,27 @@ pub mod unistd;
7979

8080
use libc::{c_char, PATH_MAX};
8181

82-
use std::{error, fmt, ptr, result};
82+
use std::{ptr, result};
8383
use std::ffi::{CStr, OsStr};
8484
use std::os::unix::ffi::OsStrExt;
8585
use std::path::{Path, PathBuf};
8686

8787
use errno::Errno;
8888

8989
/// Nix Result Type
90-
pub type Result<T> = result::Result<T, Error>;
90+
pub type Result<T> = result::Result<T, Errno>;
9191

92-
/// Nix Error Type
92+
/// Nix's main error type.
9393
///
94-
/// The nix error type provides a common way of dealing with
95-
/// various system system/libc calls that might fail. Each
96-
/// error has a corresponding errno (usually the one from the
97-
/// underlying OS) to which it can be mapped in addition to
98-
/// implementing other common traits.
99-
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
100-
pub enum Error {
101-
Sys(Errno),
102-
InvalidPath,
103-
/// The operation involved a conversion to Rust's native String type, which failed because the
104-
/// string did not contain all valid UTF-8.
105-
InvalidUtf8,
106-
/// The operation is not supported by Nix, in this instance either use the libc bindings or
107-
/// consult the module documentation to see if there is a more appropriate interface available.
108-
UnsupportedOperation,
109-
}
110-
111-
impl Error {
112-
/// Convert this `Error` to an [`Errno`](enum.Errno.html).
113-
///
114-
/// # Example
115-
///
116-
/// ```
117-
/// # use nix::Error;
118-
/// # use nix::errno::Errno;
119-
/// let e = Error::from(Errno::EPERM);
120-
/// assert_eq!(Some(Errno::EPERM), e.as_errno());
121-
/// ```
122-
pub fn as_errno(self) -> Option<Errno> {
123-
if let Error::Sys(e) = self {
124-
Some(e)
125-
} else {
126-
None
127-
}
128-
}
129-
130-
/// Create a nix Error from a given errno
131-
pub fn from_errno(errno: Errno) -> Error {
132-
Error::Sys(errno)
133-
}
134-
135-
/// Get the current errno and convert it to a nix Error
136-
pub fn last() -> Error {
137-
Error::Sys(Errno::last())
138-
}
139-
140-
/// Create a new invalid argument error (`EINVAL`)
141-
pub fn invalid_argument() -> Error {
142-
Error::Sys(Errno::EINVAL)
143-
}
144-
145-
}
146-
147-
impl From<Errno> for Error {
148-
fn from(errno: Errno) -> Error { Error::from_errno(errno) }
149-
}
150-
151-
impl From<std::string::FromUtf8Error> for Error {
152-
fn from(_: std::string::FromUtf8Error) -> Error { Error::InvalidUtf8 }
153-
}
154-
155-
impl error::Error for Error {}
156-
157-
impl fmt::Display for Error {
158-
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
159-
match *self {
160-
Error::InvalidPath => write!(f, "Invalid path"),
161-
Error::InvalidUtf8 => write!(f, "Invalid UTF-8 string"),
162-
Error::UnsupportedOperation => write!(f, "Unsupported Operation"),
163-
Error::Sys(errno) => write!(f, "{:?}: {}", errno, errno.desc()),
164-
}
165-
}
166-
}
94+
/// It's a wrapper around Errno. As such, it's very interoperable with
95+
/// [`std::io::Error`], but it has the advantages of:
96+
/// * `Clone`
97+
/// * `Copy`
98+
/// * `Eq`
99+
/// * Small size
100+
/// * Represents all of the system's errnos, instead of just the most common
101+
/// ones.
102+
pub type Error = Errno;
167103

168104
pub trait NixPath {
169105
fn is_empty(&self) -> bool;
@@ -217,7 +153,7 @@ impl NixPath for CStr {
217153
where F: FnOnce(&CStr) -> T {
218154
// Equivalence with the [u8] impl.
219155
if self.len() >= PATH_MAX as usize {
220-
return Err(Error::InvalidPath);
156+
return Err(Error::from(Errno::ENAMETOOLONG))
221157
}
222158

223159
Ok(f(self))
@@ -238,11 +174,11 @@ impl NixPath for [u8] {
238174
let mut buf = [0u8; PATH_MAX as usize];
239175

240176
if self.len() >= PATH_MAX as usize {
241-
return Err(Error::InvalidPath);
177+
return Err(Error::from(Errno::ENAMETOOLONG))
242178
}
243179

244180
match self.iter().position(|b| *b == 0) {
245-
Some(_) => Err(Error::InvalidPath),
181+
Some(_) => Err(Error::from(Errno::EINVAL)),
246182
None => {
247183
unsafe {
248184
// TODO: Replace with bytes::copy_memory. rust-lang/rust#24028

src/mount/bsd.rs

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use crate::{
2+
Error,
23
Errno,
34
NixPath,
45
Result,
@@ -99,7 +100,7 @@ libc_bitflags!(
99100
/// by `nmount(2)`.
100101
#[derive(Debug)]
101102
pub struct NmountError {
102-
errno: Errno,
103+
errno: Error,
103104
errmsg: Option<String>
104105
}
105106

@@ -109,14 +110,14 @@ impl NmountError {
109110
self.errmsg.as_deref()
110111
}
111112

112-
/// Returns the inner [`Errno`]
113-
pub fn errno(&self) -> Errno {
113+
/// Returns the inner [`Error`]
114+
pub fn error(&self) -> Error {
114115
self.errno
115116
}
116117

117-
fn new(errno: Errno, errmsg: Option<&CStr>) -> Self {
118+
fn new(error: Error, errmsg: Option<&CStr>) -> Self {
118119
Self {
119-
errno,
120+
errno: error,
120121
errmsg: errmsg.map(CStr::to_string_lossy).map(Cow::into_owned)
121122
}
122123
}
@@ -136,7 +137,7 @@ impl fmt::Display for NmountError {
136137

137138
impl From<NmountError> for io::Error {
138139
fn from(err: NmountError) -> Self {
139-
io::Error::from_raw_os_error(err.errno as i32)
140+
err.errno.into()
140141
}
141142
}
142143

@@ -381,7 +382,7 @@ impl<'a> Nmount<'a> {
381382
Some(CStr::from_bytes_with_nul(sl).unwrap())
382383
}
383384
};
384-
Err(NmountError::new(error.as_errno().unwrap(), errmsg))
385+
Err(NmountError::new(error.into(), errmsg))
385386
}
386387
}
387388
}

0 commit comments

Comments
 (0)