Use i32 in SystemError::Unknown instead of nix Errno

[ids1024]

This way users of drm can match on constants from libc, or can convert it to the error types from any version of nix or rustix. This addresses a bit of awkwardness in Smithay/smithay#1162 and rust-windowing/softbuffer#164.

This should eliminate nix as a "public dependency" of drm, though it's still exposed by drm-ffi.

[notgull]

Maybe std::io::Error would be better?

[ids1024]

Perhaps. It looks like it is possible to losslessly convert to an io::Error and back with io::Error::from_raw_os_err(e).raw_os_error().unwrap(). But that does require an unwrap() or otherwise handling None, and it's not particularly convenient if you want to compare against particular unix error codes.

[Slabity]

Could we make the entire struct #[repr(i32)] and assign the values? This struct is mostly just a small abstraction to convert those i32 values into something more DRM-focused anyways.

That way people can easily convert it into whatever format they want.

[MarijnS95]

Indeed, if this is "just wrapping errno", replacing SystemError with io::Error via io::Result<T> seems like the most natural approach via from_raw_os_error(). That is also what I did in https://github.com/rust-mobile/ndk/pull/292/files.

I'm not seeing why the unwrap is necessary: from_raw_os_error() takes a RawOsError (typedef'd as i32) and raw_os_error() returns that in an Option<RawOsError> again, defeating the purpose of having an io::Error?

[ids1024]

It wouldn't be possible to make it a #[repr(i32)] enum and still have an Unknown variant like this, since there's no way to ensure the i32 in the unknown variant isn't one of the explicit enum variants.

Rustix's Errno is a #[repr(transparent)] struct containing a c_int with associated constants for the known error types. But it doesn't seem to be possible to have a struct that's #[repr(i32)] and allows casting with as.

[ids1024]

Indeed, if this is "just wrapping errno", replacing SystemError with io::Error via io::Result seems like the most natural approach

It looks like InvalidFileType is unused. And UnknownFourcc is an error from the drm_fourcc crate, not an IO error. Otherwise these are just aliases for errno values. It may be possible to wrap the unknown fourcc error with io::Error::new, but that's a bit awkward and not really accurate to call it an "IO error".

I'm not seeing why the unwrap is necessary: from_raw_os_error() takes a RawOsError (typedef'd as i32) and raw_os_error() returns that in an Option again, defeating the purpose of having an io::Error?

I mean if the user of the library wants to handle the error, it may need to call raw_os_error() to deal with the underlying error, and has to consider the None case; but I guess it could just match on Some(ERROR_YOU_WANT_TO_TEST_FOR).

The concern about matching on the error being awkward is reduced if the whole SystemError enum could just be replace with io::Error.

[MarijnS95]

I guess one would defer to .kind() to match on the type of io::Error.

Indeed, if there are non-errno values in SystemError which I missed, it makes more sense to retain a wrapper enum for that?

[MarijnS95]

Or even better, it is likely a significantly reduced part of the API that is able to return an InvalidFourCc or InvalidFileType error, so most of the functions could return io::Result<T> and only few need to return a custom type (optionally encompassing io::Error)?

That is (again) something I've also done that way in the ndk: https://github.com/rust-mobile/ndk/pull/438/files#diff-5b2b3fe248dc9e2d1e48c16ec4e228a6319be5d167dda15a40547fbac1a7bb71R428-R435

[ids1024]

Yeah. UnknownFourcc seems to only be an error returned by Device::get_planar_framebuffer.

I guess one would defer to .kind() to match on the type of io::Error.

Smithay matches on EINTR and EBUSY. https://doc.rust-lang.org/std/io/enum.ErrorKind.html#variant.ResourceBusy isn't currently stabilized, so it would still have to compare the result of raw_os_error() for the same behavior until then.

That's the kind of thing that makes io::Error a bit awkward for low-level OS-specific things. And why both nix and rustix have their own Errno types. But yeah, using io::Error is probably overall best.

[ids1024]

Closing in favor of #173 using io::Error, plus a GetPlanarFramebufferError, since that seems to be the solution we've agreed on.