Refactor UnixAddr #1618
Refactor UnixAddr #1618
Conversation
Within UnixAddr, replace the path_len variable (length of the sun_path field) with sun_len (length of the whole structure). This is more similar to how other sockaddr types work, and it's the same way that the BSDs use the sun_len field. Also, don't require that sun_path be nul-terminated. The OS doesn't require it.
|
Oh this looks good! I think it's kinda what I was going for (though it's a little hard to remember at this point) but obviously you've looked more into the issue |
|
So the test failures on Linux are due to a Linux kernel bug. The Illumos failure is my fault and easy to fix. |
| Self::Pathname(Path::new(OsStr::from_bytes(pathname))) | ||
| let pathname = slice::from_raw_parts(sun.sun_path.as_ptr() as *const u8, path_len); | ||
| if pathname.last() == Some(&0) { | ||
| // A trailing NUL is not considered part of the path, and it does |
There was a problem hiding this comment.
Nit: you're using both NUL and nul for b'\0'. Pick one.
| unsafe { | ||
| let sun = *(addr as *const _ as *const sockaddr_un); | ||
| Ok(SockAddr::Unix(UnixAddr::from_raw_parts(sun, pathlen))) | ||
| let sun_len = len.try_into().unwrap(); |
There was a problem hiding this comment.
Unwrapping here makes me a little concerned, but I'm not familiar with the previous behavior. Should we return an Err instead of panicking?
There was a problem hiding this comment.
The unwrap is guaranteed to pass, because of the assertion on line 1914.
|
@asomers GitHub says you live in Longmont, so stay safe re: wildfires! |
On BSD-derived operating systems, struct sockaddr has a sa_len field that holds the length of the structure. UnixAddr's path_len field is redundant. Remove path_len on BSD-derived OSes, retaining it only for Illumos and Linux-based OSes. Also, ensure that two UnixAddrs compare equal if they differ only by the presence of a trailing NUL. On Linux, syscalls like getsockname add a trailing NUL to the sockaddr they return, even if no NUL was present on the sockaddr originally passed to the kernel via a syscall like bind, and even though the docs explicitly say that any NUL passed to bind is not considered to be part of the address. Work around this bug by stripping it in UnixAddrKind::get(), so that at least two UnixAddrs will compare identical even if they differ in the presence of a trailing NUL.
Fortunately the big one is downwind of me. There's a closer one upwind, but it's controlled now, and didn't do much damage. And oh yeah, it's SNOWING NOW!!! bors r+ |
Within UnixAddr, replace the path_len variable (length of the sun_path
field) with sun_len (length of the whole structure). This is more
similar to how other sockaddr types work, and it's the same way that the
BSDs use the sun_len field.
Also, don't require that sun_path be nul-terminated. The OS doesn't
require it.
On BSD-derived operating systems, struct sockaddr has a sa_len field
that holds the length of the structure. UnixAddr's path_len field is
redundant. Remove path_len on BSD-derived OSes, retaining it only for
Illumos and Linux-based OSes.