Fix nix on FreeBSD #396
Fix nix on FreeBSD #396
Conversation
|
Grr, the tests failed at runtime on OSX. The problem isn't obvious from the console output and I don't have an OSX machine to develop on. Could somebody who does take a look at the changes? Something funny is going on, because by inspection sendmsg/recvmsg never should've worked on 64-bit OSX in the first place. |
| @@ -37,7 +37,7 @@ pub struct cmsghdr { | |||
| pub cmsg_len: type_of_cmsg_len, | |||
| pub cmsg_level: c_int, | |||
| pub cmsg_type: c_int, | |||
| pub cmsg_data: [type_of_cmsg_len; 0] | |||
| pub cmsg_data: [usize; 0] | |||
There was a problem hiding this comment.
Instead of using a standard rust type here and thus affecting (though probably not breaking) other platforms, you should fix the type definition in line 14 using the appropriate type alias from libc. According to your commit message that would be socklen_t, which is defined by libc.
|
I left my comments in the code. Thank you for the changes and fixes. Regarding the failing test, I have the same problem as you. |
I have a machine running OSX and can take a look. |
|
Also, CC @geofft who initially added support. |
| @@ -37,7 +37,7 @@ pub struct cmsghdr { | |||
| pub cmsg_len: type_of_cmsg_len, | |||
| pub cmsg_level: c_int, | |||
| pub cmsg_type: c_int, | |||
| pub cmsg_data: [usize; 0] | |||
| pub cmsg_data: [size_t; 0] | |||
There was a problem hiding this comment.
Is this correct for all platforms? Or similarly, was using type_of_cmsg_len totally wrong?
There was a problem hiding this comment.
type_of_cmsg_len for cmsg_data was totally wrong. If you look at CMSG_DATA on Linux, it basically says that the data field begins at a location suitable for an aligned struct cmsghdr. On FreeBSD, CMSG_DATA basically says that the data field should begin at a location suitably aligned for a register_t, which is either a __int32_t or an __int64_t depending on the platform.
|
I started looking at fixing this for OSX and it looks like our best bet for fixing is probably to rely a bit more heavily on the (presumably correct) ffi definitions for things like Basically, I think most everything in |
|
I'll be getting access to a MAC later this evening, and I'll try out your suggestion. |
|
Changes look good to me. We can probably get rid of some code (leverage libc further) in the future, but I don't want to gait this PR on that action. Would you mind rebasing and squashing a few of the fixup commits? Things have gotten a little bit messy at this point. Once that is done, r=me. |
|
Sure. But what does "r=me" mean? |
|
Sorry, that is just code to other core team members that if they get around to looking at this first and that action has been taking, they may tell the @homu bot that it has been reviewed by me (and the bot will then merge the commit after doing some final checks). |
Force using the constants even on x86 where they do not fit into isize (c_int)
This reverts commit 012c662.
- Added initial CHANGELOG.md with Changes from Version 0.6.0 and the upcoming changes for version 0.7.0. - Added RELEASE_PROCEDURE.md detailing what changes should be made to CHANGELOG.md during a release. - Added sections about CHANGELOG.md in CONVENTIONS.md and CONTRIBUTIONS.md.
Rules for generic types were located above rules for specific types, so the rules for specific types never got matched. This caused the sys::socket::sockopt::test::can_get_listen_on_tcp_socket test to fail on FreeBSD. The solution is to put all of the generic rules at the bottom.
On Linux, the cmsg_len field of struct cmsghdr has type size_t, but it has size socklen_t on POSIX-compliant operating systems. So on POSIX-compliant 64-bit operating systems, struct cmsghdr has padding gaps that aren't present on Linux. Most of the issues fixed by this commit related to those gaps. src/sys/socket/ffi.rs Fix the type of the cmsg_data field so the struct layout will be correct. src/sys/socket/mod.rs In CmsgIterator.next, only return a single file descriptor. sendmsg(2) can only stuff a single file descriptor into each cmsg. In cmsg_align, fix the rounding calculation, and eliminate a division instruction. Add a missing cmsg_align call in ControlMessage.len In ControlMessage.encode_into, add any necessary padding bytes between the cmsghdr and the data. In sendmsg, fix some len<->capacity confusion.
|
I cleaned up the PR in #397 and merged it. Thank you. |
|
One more thing @asomers: could you update CHANGELOG.md regarding new features, breaking changes, and fixes? |
No description provided.