Errno::result()

[arcnmx]

A replacement of from_ffi, and updates all methods to use it. Partial preparation for #230 where Error will be replaced by Errno.

Changes are on top of #246, so merge that first!

[kamalmarhubi]

I'm going to take a look at this tomorrow. I also think we'll want input from an extra reviewer or two since it's a major change to the API.

[arcnmx]

Well, to be fair the only API change is renaming from_ffi to Errno::result :P

But yeah, it touches a lot of the codebase! Not sure who else to pull in.

[kamalmarhubi]

Well, to be fair the only API change is renaming from_ffi to Errno::result :P

I thought this was changing the return type of every function in the library from ::std::result::Result<_, ::nix::Error> to ::std::result::Result<_, ::nix::errno::Errno>? Am I confused?

[arcnmx]

No, this doesn't change anything externally (except from_ffi, which is a public function). Signatures are otherwise all the same, and nix::Error is still used because this doesn't include the NixString changes (and therefore InvalidPath is still an applicable error).

[kamalmarhubi]

My confusion stemmed from moving this into the errno module. What's the rationale?

[arcnmx]

Oh, it should probably be moved back. Mostly it's there because that's where #230 had it.

[kamalmarhubi]

Mind moving it back? :-)

[arcnmx]

Yeah, I'll do it when I get a chance sometime tomorrow night. Don't let that keep you from reviewing the rest of it though!

[kamalmarhubi]

Based on what you said, and when I'm more awake, I'm pretty sure this will be fine. I'd been meaning to do a from_ffi sweep soon, so...

[kamalmarhubi]

Ah, I was confused. See inline comment #247 (comment)

[arcnmx]

Essentially, this is just a PR to change every function to use from_ffi. It also just happens to rename it to Errno::result(), for two reasons:

1. It's more self-documenting, etc. It returns a nix::Result from a POSIX function return value containing an Errno.
2. It prepares for the change that actually does swap nix::Result to Result<T, Errno>. None of the code this touches will need to be changed as part of NixPath -> NixString and Error -> Errno #230 (which will actually change the function signatures).

[kamalmarhubi]

Thanks for the clarification!

[arcnmx]

Moved Result back.

[kamalmarhubi]

CI failed on apple. Please apply:

diff --git a/src/sys/event.rs b/src/sys/event.rs
index eb99f7d..1563a74 100644
--- a/src/sys/event.rs
+++ b/src/sys/event.rs
@@ -1,7 +1,8 @@
 /* TOOD: Implement for other kqueue based systems
  */

-use errno::{Errno, Result};
+use Result;
+use errno::Errno;
 #[cfg(not(target_os = "netbsd"))]
 use libc::{timespec, time_t, c_int, c_long, uintptr_t};
 #[cfg(target_os = "netbsd")]

:-)

[kamalmarhubi]

Ok I'm going over this once more and then let's get the damned thing merged!

[posborne]

Did a quick review. I'm happy with this change.

[kamalmarhubi]

Rebased and merged as 01e8416...f167e8f