Converts AddressFamily to struct with associated consts
#2210
Conversation
Jan561
force-pushed
the
address_families
branch
2 times, most recently
from
1e05db3
to
d47caef
Compare
|
I'll provide a changelog when rebasing on top of #2211 |
Jan561
force-pushed
the
address_families
branch
5 times, most recently
from
3ae5caf
to
46c0af3
Compare
|
If you want, I can just add the new variants to the enum for the time being, and postpone the breaking changes |
Jan561
force-pushed
the
address_families
branch
from
46c0af3
to
70175c4
Compare
| apple_targets, | ||
| freebsdlike, | ||
| linux_android, | ||
| netbsdlike, |
There was a problem hiding this comment.
| apple_targets, | |
| freebsdlike, | |
| linux_android, | |
| netbsdlike, | |
| bsd, | |
| linux_android, |
| @@ -351,7 +552,7 @@ impl UnixAddr { | |||
| pub fn new<P: ?Sized + NixPath>(path: &P) -> Result<UnixAddr> { | |||
| path.with_nix_path(|cstr| unsafe { | |||
| let mut ret = libc::sockaddr_un { | |||
| sun_family: AddressFamily::Unix as sa_family_t, | |||
| sun_family: AddressFamily::UNIX.family() as sa_family_t, | |||
There was a problem hiding this comment.
Should AddressFamily implement Into<sa_family_t>?
|
|
||
| /// Address families, corresponding to `AF_*` constants in libc. | ||
| #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] | ||
| pub struct AddressFamily(libc::c_int); |
There was a problem hiding this comment.
All Newtypes should use repr(transparent). You might even be able to cast directrly from AddressFamily to other int types just by using as, instead of .family().
| pub struct AddressFamily(libc::c_int); | |
| #[repr(transparent)] | |
| pub struct AddressFamily(libc::c_int); |
| } | ||
|
|
||
| /// Returns the c_int representation of the address family. | ||
| pub const fn family(&self) -> libc::c_int { |
There was a problem hiding this comment.
This method is confusingly named, since Self is already a "family". Better would be to implement Something like Into<libc::c_int>.
| @@ -689,7 +689,7 @@ pub enum ControlMessageOwned { | |||
| /// // Set up | |||
| /// let message = "Ohayō!".as_bytes(); | |||
| /// let in_socket = socket( | |||
| /// AddressFamily::Inet, | |||
| /// AddressFamily::INET, | |||
There was a problem hiding this comment.
All of these capitalization changes would be a burden on our users. It would be much more convenient if we didn't change it.
It should've been a struct with associated constants from the beginning. At least theoretically. I won't ever need vendor address families personally, as well as 99.9% of nix users. So yeah, the practical impact of this PR as is would be very low. That's why I was suggesting to change the PR to only add new variants to the enum (I only need to change one line of my script). But before hitting 1.0 (in the very distant future), I would definitely reconsider the current enum API. |
Split off from #2199
Script I used to generate the variants: https://gist.github.com/Jan561/067d64641c93017a9bf6b346ac078ec1
Checklist:
CONTRIBUTING.md