Add mq_getfd_np() on FreeBSD and fix mqd_t on DragonFlyBSD #1308
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @gnzlbg (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
|
The change looks correct to me. I don't think that it should be considered a breaking change when the old definitions were simply wrong. There is precedent within libc for fixing incorrect definitions. Oh, and posixmq looks cool! |
|
@bors: r+ |
|
📌 Commit 652b832 has been approved by |
Add mq_getfd_np() on FreeBSD and fix mqd_t on DragonFlyBSD [`mq_getfd_np()` was added in FreeBSD 11](https://svnweb.freebsd.org/base/stable/11/include/mqueue.h?revision=306905&view=markup). I'm already using it in my [posixmq crate](https://github.com/tormol/posixmq) for mio/kqueue integration, and I've tested it both in virtualbox and on Cirrus-CI. [`mqd_t` in an `int` on DragonFlyBSD](https://github.com/DragonFlyBSD/DragonFlyBSD/blob/e7ab884bd49753f8884eb597d10d6569a08fa0df/sys/sys/types.h#L139) even though it's a pointer on FreeBSD, because [DragonflyBSD's implementation is based on NetBSD](DragonFlyBSD/DragonFlyBSD@f2df0f7). The definitions for `mq_attr` are already separate and correct. Does fixing this count as a breaking change? I think the current definition will work in most cases, because IIRC the calling convention means that `mqd_t` is always passed via registers, the upper 32 bits might just contain garbage. I've *not* tested this change on DragonFlyBSD. I want to add mq symbols for solarish, but is the CDDL license compatible with Apache-2.0 and MIT?
|
@strangelittlemonkey: 🔑 Insufficient privileges: Not in reviewers |
|
@strangelittlemonkey that would be great! |
|
☀️ Test successful - checks-cirrus, checks-travis, status-appveyor |
|
I got around to compiling and things are broken on DragonFly again. The breaking commit seems to have been 7ac0fe5. Looks like it needs some fixes to casting again. |
|
@strangelittlemonkey by broken you mean the libc-test right? not the build, right? That commit seems unrelated to this one :) |
|
zach@dev03 ~/libc> cargo build error[E0308]: mismatched types error[E0308]: mismatched types error[E0308]: mismatched types error: aborting due to 4 previous errors For more information about this error, try To learn more, run the command again with --verbose. Whether or not libc-test passes, if the build fails I can't use it as a lib in a project :) |
|
You have an old version of libc. This PR was released as part of 0.2.51
I installed dragonflybsd in a VM today to test my crate, and it compiled.
Den 30. mars 2019 19:03:41 CET, skrev Zach Crownover <notifications@github.com>:
…***@***.*** ~/libc> cargo build
Compiling libc v0.2.49 (/home/zach/libc)
error[E0308]: mismatched types
--> src/unix/bsd/freebsdlike/dragonfly/mod.rs:807:9
|
806 | pub fn CMSG_LEN(length: ::c_uint) -> ::c_uint {
| -------- expected `u32`
because of return type
807 | _CMSG_ALIGN(::mem::size_of::<::cmsghdr>()) + length as
usize
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
expected u32, found usize
error[E0308]: mismatched types
--> src/unix/bsd/freebsdlike/dragonfly/mod.rs:813:48
|
813 | let next = cmsg as usize + _CMSG_ALIGN((*cmsg).cmsg_len)
| ^^^^^^^^^^^^^^^^
expected usize, found u32
error[E0308]: mismatched types
--> src/unix/bsd/freebsdlike/dragonfly/mod.rs:818:42
|
818 | (cmsg as usize + _CMSG_ALIGN((*cmsg).cmsg_len)) as
*mut ::cmsghdr
| ^^^^^^^^^^^^^^^^ expected
usize, found u32
error[E0308]: mismatched types
--> src/unix/bsd/freebsdlike/dragonfly/mod.rs:825:9
|
824 | pub fn CMSG_SPACE(length: ::c_uint) -> ::c_uint {
| -------- expected `u32`
because of return type
825 | / _CMSG_ALIGN(::mem::size_of::<::cmsghdr>()) +
826 | | _CMSG_ALIGN(length as usize)
| |________________________________________^ expected u32, found usize
error: aborting due to 4 previous errors
For more information about this error, try `rustc --explain E0308`.
error: Could not compile `libc`.
To learn more, run the command again with --verbose.
Whether or not libc-test passes, if the build fails I can't use it as a
lib in a project :)
--
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#1308 (comment)
--
Sendt fra min Android-enhet med K-9 e-post. Unnskyld min kortfattethet.
|
|
That was master branch
… On Mar 30, 2019, at 12:16 PM, Torbjørn Birch Moltu ***@***.***> wrote:
You have an old version of libc. This PR was released as part of 0.2.51
I installed dragonflybsd in a VM today to test my crate, and it compiled.
Den 30. mars 2019 19:03:41 CET, skrev Zach Crownover ***@***.***>:
***@***.*** ~/libc> cargo build
> Compiling libc v0.2.49 (/home/zach/libc)
>error[E0308]: mismatched types
> --> src/unix/bsd/freebsdlike/dragonfly/mod.rs:807:9
> |
>806 | pub fn CMSG_LEN(length: ::c_uint) -> ::c_uint {
>| -------- expected `u32`
>because of return type
>807 | _CMSG_ALIGN(::mem::size_of::<::cmsghdr>()) + length as
>usize
>| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>expected u32, found usize
>
>error[E0308]: mismatched types
> --> src/unix/bsd/freebsdlike/dragonfly/mod.rs:813:48
> |
>813 | let next = cmsg as usize + _CMSG_ALIGN((*cmsg).cmsg_len)
>| ^^^^^^^^^^^^^^^^
>expected usize, found u32
>
>error[E0308]: mismatched types
> --> src/unix/bsd/freebsdlike/dragonfly/mod.rs:818:42
> |
>818 | (cmsg as usize + _CMSG_ALIGN((*cmsg).cmsg_len)) as
>*mut ::cmsghdr
>| ^^^^^^^^^^^^^^^^ expected
>usize, found u32
>
>error[E0308]: mismatched types
> --> src/unix/bsd/freebsdlike/dragonfly/mod.rs:825:9
> |
>824 | pub fn CMSG_SPACE(length: ::c_uint) -> ::c_uint {
>| -------- expected `u32`
>because of return type
>825 | / _CMSG_ALIGN(::mem::size_of::<::cmsghdr>()) +
>826 | | _CMSG_ALIGN(length as usize)
> | |________________________________________^ expected u32, found usize
>
>error: aborting due to 4 previous errors
>
>For more information about this error, try `rustc --explain E0308`.
>error: Could not compile `libc`.
>
>To learn more, run the command again with --verbose.
>
>
>Whether or not libc-test passes, if the build fails I can't use it as a
>lib in a project :)
>
>--
>You are receiving this because you authored the thread.
>Reply to this email directly or view it on GitHub:
>#1308 (comment)
--
Sendt fra min Android-enhet med K-9 e-post. Unnskyld min kortfattethet.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
@strangelittlemonkey from your build output (emphasis mine):
If it were master branch, it would have said |
|
Well, now that I’ve thoroughly embarrassed myself, you were right it was my fork. I updated it. Output as follows:
zach@dev03 ~/l/libc-test> cargo test
Compiling libc v0.2.51 (/home/zach/libc)
Compiling log v0.3.9
Compiling quote v0.6.11
Compiling extprim v1.6.0
Compiling rand v0.4.6
Compiling syn v0.15.26
Compiling serde_derive v1.0.87
Compiling serde v1.0.87
Compiling syntex_pos v0.59.1
Compiling serde_json v1.0.38
Compiling syntex_errors v0.59.1
Compiling syntex_syntax v0.59.1
Compiling ctest v0.2.8
Compiling libc-test v0.1.0 (/home/zach/libc/libc-test)
error[E0432]: unresolved import `ctest::VolatileItemKind`
--> libc-test/build.rs:909:20
|
909 | use ctest::VolatileItemKind::*;
| ^^^^^^^^^^^^^^^^ could not find `Vo$atileItemKind` in `ctest`
error[E0531]: cannot find tuple struct/variant `StructField` in this scope
--> libc-test/build.rs:911:13
|
911 | StructField(ref n, ref f) if n == "aiocb" && f == "aio_buf" => {
| ^^^^^^^^^^^ not found in this scope
error: unused import: `ctest::VolatileItemKind::*`
--> libc-test/build.rs:909:13
|
909 | use ctest::VolatileItemKind::*;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: lint level defined here
--> libc-test/build.rs:1:9
|
1 | #![deny(warnings)]
| ^^^^^^^^
= note: #[deny(unused_imports)] implied by #[deny(warnings)]
error[E0599]: no method named `volatile_item` found for type `ctest::TestGenerator` in the current scope
--> libc-test/build.rs:908:9
|
908 | cfg.volatile_item(|i| {
| ^^^^^^^^^^^^^
error: aborting due to 4 previous errors
Some errors occurred: E0432, E0531, E0599.
For more information about an error, try `rustc --explain E0432`.
error: Could not compile `libc-test`.
To learn more, run the command again with --verbose.
… On Mar 30, 2019, at 12:36 PM, gnzlbg ***@***.***> wrote:
@strangelittlemonkey from your build output (emphasis mine):
***@***.*** ~/libc> cargo build
Compiling libc v0.2.49 (/home/zach/libc)
If it were master branch, it would have said v0.2.51 instead. Maybe you pulled the master branch from an old fork?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
I think you might be picking up a too old ctest as well.. |
|
Ok, to be sure I cloned a clean copy and ran it. zach@dev03 ~/l/libc-test> cargo test
|
|
So it appears that, at least on your dragonflybsd version:
which DragonflyBSD version are you using @strangelittlemonkey ? @tormol which DragonflyBSD version did you test? |
|
I wasn't testing libc other than using it as a dependency. |
mq_getfd_np()was added in FreeBSD 11. I'm already using it in my posixmq crate for mio/kqueue integration, and I've tested it both in virtualbox and on Cirrus-CI.mqd_tin aninton DragonFlyBSD even though it's a pointer on FreeBSD, because DragonflyBSD's implementation is based on NetBSD. The definitions formq_attrare already separate and correct.Does fixing this count as a breaking change? I think the current definition will work in most cases, because IIRC the calling convention means that
mqd_tis always passed via registers, the upper 32 bits might just contain garbage.I've not tested this change on DragonFlyBSD.
I want to add mq symbols for solarish, but is the CDDL license compatible with Apache-2.0 and MIT?