Print out errname in impl Debug for Error

[nbdd0121]

Depends on #273 Merged

[fbq]

I like the idea that we could have names instead of values for the Debug::fmt of Error. However, I think it's better that we don't need to call errname if we already have the corresponding const, e.g. Error::EINVAL, because having the const actually means we have names for the error numbers at Rust side.

Thoughts?

[nbdd0121]

We don't currently have constants for all errors defined in Rust. Plus, defining them in Rust essentially requires us to have a giant match, or define a lookup table like errname.c did. It might be better just to reuse errname, C side needs it anyway.

[nbdd0121]

I think there are a few further improvements that can be made to Error.

1. We might want to drop all negatives. All C side macros define error codes as positive, and it's only used as negative values to tell part between success and failure. Rust side shouldn't use that pattern at all and should use Result exclusively anyway, so negative values are not meaningful.
2. Error should #[derive(Clone, Copy, PartialEq, Eq)] included in the PR
3. We will need to make sure that no Error with out of range error code can be constructed (e.g. 0, > MAX_ERROR), as otherwise it might be a soundness hole (with the use of from_kernel_result for example).
4. After 3 is fixed, we can probably use NonZeroI16 as the inner type. This allows Result<()> to fit in 16-bit, instead of 64-bit as it is now.

[TheSven73]

4. After 3 is fixed, we can probably use NonZeroI16 as the inner type. This allows Result<()> to fit in 16-bit, instead of 64-bit as it is now.

I like this. Moving to a more constrained type would eliminate at least one human "proof" elsewhere. That's probably a Good Thing(tm).

[ksquirrel]

Review of 34c1d99a247f:

• ✔️ Commit 34c1d99: Looks fine!

[ojeda]

LGTM.

[ojeda]

We could put some of #287 (comment) as an issue, perhaps a "good first issue".