Add support for system control sockets for XNU #478
Conversation
|
Do you think you'll be able to review this, @asomers? |
There was a problem hiding this comment.
You should remove the Linux restriction in test/sys/test_ioctl.rs and make it pass on the BSDs. Also, you should remove the restriction against FreeBSD and Dragonfly in src/sys/mod.rs. Other than that, it looks fine. I flagged a few things inline, but they aren't problems introduced by you.
| #[macro_export] | ||
| macro_rules! ioctl { | ||
| (bad $name:ident with $nr:expr) => ( | ||
| pub unsafe fn $name(fd: $crate::sys::ioctl::libc::c_int, |
There was a problem hiding this comment.
This seems to be overloading the "bad" keyword. AFAICT, in Linux the difference between _IOR and _IOR_BAD is in the interpretation of the size argument. However, the bad macro here deals with callers who hard-code the ioctl number. I know @conradev didn't introduce the bad keyword, but it would be nice to fix
There was a problem hiding this comment.
I removed the bad keyword so that callers who want to hardcode the ioctl number now should provide no keyword. Does that seem like an appropriate solution?
| #[macro_export] | ||
| macro_rules! ioc { | ||
| ($inout:expr, $group:expr, $num:expr, $len:expr) => ( | ||
| $inout | (($len as u32 & $crate::sys::ioctl::IOCPARM_MASK as u32) << 16) | (($group as u32) << 8) | ($num as u32) |
There was a problem hiding this comment.
The second as u32 is superfluous, since IOCPARAM_MASK is already defined as u32.
There was a problem hiding this comment.
As a sidenote, if the trivial_numeric_casts lint was on, this would've been caught 😮
| @@ -12,7 +12,8 @@ pub mod eventfd; | |||
| #[cfg(target_os = "linux")] | |||
| pub mod memfd; | |||
|
|
|||
| #[cfg(not(any(target_os = "ios", target_os = "freebsd", target_os = "dragonfly")))] | |||
| #[cfg(not(any(target_os = "freebsd", target_os = "dragonfly")))] | |||
There was a problem hiding this comment.
It's fine to get rid of the "freebsd" here. Probably fine to get rid of dragonfly, too, but I can't test on that platform.
|
Made those changes! Let me know if you have further feedback :) |
|
Could you rebase to master and push again (with -f), so that the CI is green? Could you also add an entry to our CHANGELOG.md to document the changes. After that we can merge it. |
|
Rebased against master. I documented the additions, and the one existing API change! |
|
@fiveop, let me know if I need to make additional changes to get this merged 😸 |
|
@homu r+ thanks |
|
📌 Commit ef257e0 has been approved by |
Add support for system control sockets for XNU I added support for macOS and iOS system sockets, which can be used to control the kernel as described [here](https://developer.apple.com/library/content/documentation/Darwin/Conceptual/NKEConceptual/control/control.html). To do this, I had to add in support for `ioctl` on those platforms, so I added in `ioctl` support for all BSD-based platforms. The API seems to be the same between [xnu](https://opensource.apple.com/source/xnu/xnu-3248.60.10/bsd/sys/ioccom.h.auto.html), [FreeBSD](https://github.com/freebsd/freebsd/blob/master/sys/sys/ioccom.h), [NetBSD](https://ftp.netbsd.org/pub/NetBSD/NetBSD-current/src/sys/sys/ioccom.h), [OpenBSD](http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/sys/ioccom.h?rev=1.5&content-type=text/x-cvsweb-markup) and [Dragonfly BSD](http://gitweb.dragonflybsd.org/dragonfly.git/blob/HEAD:/sys/sys/ioccom.h). I added a test that runs on macOS and iOS for the functionality. Let me know if I need to make any changes!
|
☀️ Test successful - status |
I added support for macOS and iOS system sockets, which can be used to control the kernel as described here.
To do this, I had to add in support for
ioctlon those platforms, so I added inioctlsupport for all BSD-based platforms. The API seems to be the same between xnu, FreeBSD, NetBSD, OpenBSD and Dragonfly BSD.I added a test that runs on macOS and iOS for the functionality. Let me know if I need to make any changes!