japaric/steed

Fate of `last_os_error`

Opened this issue · 8 comments

tbu- commented

Since we don't have a C-style errno variable, what should we return from io::Error::last_os_error. Currently, it just panics.

Other options include: Introducing a C-style errno variable, or returning 0 (apparantly Redox does that, #43 (comment)).

I recommend making a Rust RFC to deprecate and remove io::Error::last_os_error from libstd.

Redox does return 0 for errno, and I think that makes the most sense here until we remove the function completely.

tbu- commented

I doubt that this functionality will be removed from the stdlib anytime soon. It's marked as stable.

Wouldn't it be safer to return any value other than 0, since 0 usually is reserved to mean "no error"? Maybe EIO instead.

I agree that it isn't likely to get removed from libstd soon, but we should start the process to finding some solution.

I'm of the opinion that steed should not provide last_os_error. That is, if your crate uses last_os_error you should get a compile error if you try to compile it against steed; then you should cfg (adapt) your code to e.g. use a Result-based syscall interface instead of libc functions when compiling against steed.

Also, retuning either 0 or other fixed value from last_os_error would be wrong. Because one always signals "last operation was successful" and the other always signals "last operation failed" regardless of what really happened.

Because one always signals "last operation was successful" and the other always signals "last operation failed" regardless of what really happened.

Isn't it a mistake to call last_os_error() if the last operation didn't fail? In C, it is a bug to look at errno unless the last operation signaled failure. libc functions MUST NOT touch errno unless there is an error. Therefore there was never an error encountered, or if non-libc code reset errno, errno will always have a stale error value in it when the last function succeeded. See https://www.securecoding.cert.org/confluence/pages/viewpage.action?pageId=6619179.

Regardless, I agree that it would be better to just not define get_last_error().

Redox now does use a real errno for this: rust-lang/rust#39212.