Removing unsafe by replacing libc with nix
Hi!
First off: thanks so much for making this project! I've learned a lot just by reading it, which is fantastic!
Observation
I was going through the source today (on the futures-0.3 branch actually :sparkles:) and something that stood out to me is that there's quite some unsafe sprinkled throughout the code to interact with the OS through libc. nix is a package which aims to make using libc's functionality safe, removing the need for unsafe.
I feel this would have a few benefits:
- It would emphasize that it's possible to write executors in (mostly) safe code.
- It's probably a bit more readable for most people (e.g. no need to worry unsafe and null pointers).
Example
For example the select(2) loop in the reactor currently looks like this:
let rv = unsafe {
select(
nfds,
&mut read_fds,
&mut write_fds,
std::ptr::null_mut(),
&mut tv,
)
};
// don't care for errors
if rv == -1 {
panic!("select()");
} else if rv == 0 {
debug!("timeout");
} else {
debug!("data available on {} fds", rv);
}
Using the nix select function this could be rewritten to something like:
// don't care for errors
let rv = select(
Some(nfds),
Some(&mut read_fds),
Some(&mut write_fds),
None,
Some(&mut tv),
).unwrap();
debug!("data available on {} fds", rv);
Which might feel a bit more natural for most Rustaceans.
Conclusion
We made a case for replacing the usage of libc with the nix crate, citing potential benefits, and created an example conversion using existing code. I hope this issue might come in helpful. Thanks for your time!
Hi,
I'm happy the project was useful for you!
Now to the point, I actually considered using nix, but decided against it for two reasons:
1.minimal dependencies
I think, in a teaching project, it makes sense to minimize dependencies, as every dependency adds cognitive load. A reader may be unfamiliar with nix, so they would have to consult nix docs. On the other hand, libc::select makes it clear that this is a well known C select function.
2.nix being huge Nix covers a lot of APIs and consists of ~20k lines + 4 dependencies. Well, even mio dropped nix because it's heavy.
So, considering this, what do you think about adding a safe wrapper around select to fahrenheit itself? It would look as safe as nix in the main file, but a curious reader could inspect it in some util.rs right here.