No conversion between Errno and std::io::Error/i32

[jethrogb]

There are situation where you want to have the raw error value of an error, e.g. for passing back into C code. Also, there is already a perfectly good type for holding errno in Rust, std::io::Error. It does support getting the raw error value. I suggest deprecating Errno in favor of std::io::Error or at least adding the necessary conversions.

[Susurrus]

There is From<Errno> for io::Error. Does this not work for your use case?

[jethrogb]

I'm sure I looked for that but couldn't find it in the documentation! I now realize that it wouldn't be in the documentation at all (rust-lang/rust#42440). In any case, this is not sufficient because errno doesn't store the original error code, so unknown codes get clobberred into 0.

[Susurrus]

Okay, I think I understand the issue here. You want the i32 of the Errno back. Could you provide a code example that demonstrates your issue?

Right now there is impl From<Errno> for io::Error that should allow you to do example what you want using .into(). Maybe that wasn't discoverable for you or is simply less ergonomic than you like?

[jethrogb]

There are two separate issues.

1. The implementation is not discoverable. This is the Rust issue I linked. I don't think there's anything you can do about this (except maybe add some explicit text to the documentation page for Errno).
2. The conversion from i32 to Errno to io::Error back to i32 is lossy:

#[test]
fn errno_conversion() {
    assert_eq!(std::io::Error::from(nix::Errno::from_i32(1234)).raw_os_error(), Some(1234));
}

	thread 'errno_conversion' panicked at 'assertion failed: `(left == right)` (left: `Some(0)`, right: `Some(1234)`)', src/lib.rs:5

[Susurrus]

Regarding the first point, I definitely agree. I think we would be better served to move all error handing into an error.rs and then provide module-level documentation explaining the various tools we provide for people to interface with the Error class.

As for the second point, that behavior is because 1234 isn't a valid errno. I actually think that function should be changed to return a Result. I don't see why nix should be supportive of unknown errnos since they're so constant.

[Susurrus]

@jethrogb Would you be willing to add some docs to the errno module explaining this conversion? As to your second point, I don't think we need to support arbitrary errnos, but maybe you have a use case we hadn't considered?

[jethrogb]

I actually think that function should be changed to return a Result.

By “that function” do you mean Errno::from_i32? I don't think that's a good idea, this function is used to convert error codes returned when interfacing with C functions. By the time you're calling Errno::from_i32 you know you have a valid errno value from somewhere else.

Errno's do occasionally change
https://github.com/torvalds/linux/blame/master/include/uapi/asm-generic/errno.h
https://github.com/opensource-apple/xnu/blame/10.12/bsd/sys/errno.h

Newly introduced error codes might not make much sense to old binaries interpreting them, but at least they can mean something to the user (either in raw form or through strerror by dynamically linking to libc). It seems like a waste to throw away this valuable error information.

[Susurrus]

@jethrogb I'd be happy to consider a more concrete solution here along if it also showed specific use cases the current system doesn't allow or are ergonomic. Please file a PR if you have an idea for what you want and think is reasonable to implement and we can iterate there I think.

[kahing]

Here's a related issue that doesn't work right now

fn io_func() -> io::Result<()> {
    std::fs::some_function()?;
    nix::some_other_function()?;
}

[Susurrus]

@kahing That shouldn't work. Not all error types are compatible between libraries, nor are they designed to be compatible. There is no appropriate mapping that works in all use cases from nix::Error to io::Error, which is why one is not provided.

In your code example, you should invent a new error type that provides a From<io::Error> and a From<nix::Error> and use that as the return type to this function.

@jethrogb Are you interested in filing a PR and driving a resolution for this issue?

[kahing]

I understand that they are not compatible but since rust io and nix are often used together, it would be nice if they were compatible out of the box. (yes, I do have my own error type now).

[jethrogb]

I think it makes a lot of sense to provide From<nix::Error> for io::Error. If the original error is an Errno, it's an obvious conversion, otherwise, it can be a io::ErrorKind::Other.

I'll look into doing a PR at some point

[Susurrus]

@jethrogb we removed it in #614, we won't be adding it back in, so need to work on a PR for this. The rust-y way to handle this is to create your own error type and map from all other errors into that error type for export.

@jethrogb If you want to file a PR for the other things we've discussed in this issue, that would definitely be considered.

[goertzenator]

I would love to see From<nix::Error> for io::Error in nix.

Reasoning: People are writing their own conversions. @paulpr0 is doing it in the reference above, and I will be doing it too. Having a good impl in nix would reduce friction for users who want to manage errors in this way.

Would the existence of From<nix::Error> for io::Error break existing code or cause other problems?

[MOZGIII]

I'm using ioctl_*! macros, and my conversion to std::io::Error is std::io::Error::from(err.as_errno().unwrap()).
This is safe with the current implementation, because it internally uses nix::Error::result, that always creates Error::Sys variant for Error:

nix/src/errno.rs

Lines 77 to 86 in a4a465d

	/// Returns `Ok(value)` if it does not contain the sentinel value. This
	/// should not be used when `-1` is not the errno sentinel value.
	pub fn result<S: ErrnoSentinel + PartialEq<S>>(value: S) -> Result<S> {
	if value == S::sentinel() {
	Err(Error::Sys(Self::last()))
	} else {
	Ok(value)
	}
	}
	}

But, in fact, there's absolutely no need for that nix::Error type to take place at for ioctl call, and bare Result<c_int, Errno> should be used instead.

The error conversion would then simply be done via ? as expected.

What I'm saying is that current API for ioctl is not designed very wisely, and can be simplified for the better. Maybe the same applies for the rest of the errors nix produces? It might be that nix::Error is actually usable only in a handful of cases, and in that sense should not be used everywhere in the nix (as an error type by default) as it is used currently.

I propose reviewing the code to find all places where using nix::Error is actually required, split it into multiple types that represent possible error conditions at a particular code more accurately (i.e. some fns can't ever return UnsupportedOperation variant, and some can't ever return InvalidPath) and rewrite the code to return the most specific (i.e. the most accurately representing actual real failure scenarios) error type.
We can then reconstruct the nix::Error type from every possible error condition to provide a one-size-fits-all type, though we might also not want to provide that because there are a lot of alternatives for the users (like using Box<dyn std::error::Error> or using failure crate). And, again, we'll only need it for the users, cause our API would be covered with more specific error types.

[ExceptionallyHandsome]

I think this now served by std::io::Error::from_raw_os_error(errno as i32), right? I mean, both match against the same libc::E* constants when round tripping to/from i32.

[jethrogb]

@ExceptionallyHandsome #613 (comment)

this is not sufficient because errno doesn't store the original error code, so unknown codes get clobberred into 0.

[coolreader18]

There will likely be an io::ErrorKind::NotSupported added in Rust 1.52 (rust-lang/rust#78880), I think that be a good mapping for nix::Error::UnsupportedOperation if From<nix::Error> for io::Error were ever to be added again.

[coolreader18]

Also, I'd like to make a case for why that impl should be added again: nix::Error is basically a subset of io::Error. While io::Error can hold an arbitrary user error of any ErrorKind or an errno, nix::Error can hold a user error of kind UnsupportedOperation, InvalidPath, InvalidUtf8 or an errno. IMO there's a very clear mapping between the latter and the former, and I don't think it makes sense to have to make a custom error type just because you have code that does both stdlib io and unix-specific io. Obviously it's a good idea to have nix::Error as its own separate type, since it vastly reduces the set of errors that can exist and it makes it much easier to handle, but if you do need to "widen" the error type, I think converting to io::Error makes more sense than having an enum Error { Nix(nix::Error), Io(io::Error) }