-
Notifications
You must be signed in to change notification settings - Fork 674
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add better support for unnamed unix socket addrs #1857
Conversation
b1af529
to
60f17d7
Compare
I decided to only enable these on Linux/Android, since unnamed unix sockets seem to behave differently on FreeBSD and I can't find any documentation for its behaviour. For example when running The current test failures seem to be unrelated, probably from a new clippy version. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that defining this for Linux only is appropriate, because AFAIK no BSD OS documents such a thing. But don't forget the CHANGELOG. Also, I'd like to merge this before PR #1871.
test/sys/test_socket.rs
Outdated
|
||
let addr_1: UnixAddr = getsockname(fd_1).expect("getsockname failed"); | ||
assert!(addr_1.is_unnamed()); | ||
assert_eq!(addr_1, UnixAddr::new_unnamed()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this part guaranteed to work, according to any Linux documentation, or is it just luck? I'm not sure we should have this assertion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Linux says "When the address of an unnamed socket is returned, its length is sizeof(sa_family_t)
", so the only possible unnamed unix socket address it could return would be sockaddr_un { sun_family: AF_UNIX, ... }
with length 2. But I agree that the documentation around this still isn't great, so I removed this assertion.
Also, rebasing should fix the CI failures. |
60f17d7
to
500baa1
Compare
Removed test assertion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but I violated my own request and merged #1871 first. So I'll fix the subtle conflict myself.
bors r+
Use it in the from_sockaddr_un_abstract_unnamed test. That test and this method were introduced by PRs nix-rust#1871 and nix-rust#1857, which crossed each other.
This adds the following 2 functions/methods:
UnixAddr::new_unnamed
andUnixAddr::is_unnamed
.Closes #1585
unix(7) on Linux:
Edit: This currently isn't working on BSD, but I see why. Will fix it shortly.