Improve documentation for ioctl

[Susurrus]

I'm working on converting my crate to use nix instead of ioctl-rs as nix is better maintained. The big issue here is that the documentation for using ioctls with nix is not good from a user's point of view. Here are some issues I ran into:

• The difference between using ioctl! and using ior!, iow!, iorw!, and ioc is not listed clearly. I'm sure one is preferred over the other, but I'm not certain which. The example only uses ioctl!.
• It should be clarified that ioctl constants should be upstreamed into libc and then those should be used as inputs/outputs. The example doesn't do that, so we'll have tons of people reimplementing everything per-crate.
• The ioctl! example only uses read and write. Looking at the macro there are 8 forms of it, but the documentation doesn't actually describe any of them.
• It's unclear how to use ioc_dir and friends. There are no docs and they don't take clear types, but there are constants that have similar names in the module, so maybe those are related?
• There are various constants that are also a part of the ioctl module, but those don't seem attached to these functions.
• nix doesn't provide helpful wrapper functions for ioctls even if they're quite common (TIOCEXCL would be an example) and well-defined. I'd suggest that wrapper functions be provided for well-defined ioctls. This may be controversial, as I think it's come up before, because ioctl's are unbounded and defined by kernel drivers. So maybe there is a role for an ioctl crate that does this on top of nix.

[Susurrus]

Even the most basic question, of how do I convert an ioctl that looks like ioctl(fd, TIOCEXCL) to a proper one using the API defined by nix isn't answered. I don't see where I'm supposed to pass a file descriptor to the macro and the example provided, even though there are 12 provided don't ever pass an fd. Looking into this further, it appears that the ioctl! macro generates unsafe functions, and those can then be called. The ioctl module docs show examples in generating the functions, but then they don't ever use it.

[Susurrus]

Yeah, there's not enough docs for me to convert a simple no-data ioctl to use nix, so I'm going to give up for now. Any help would be appreciated here.

cc @posborne as you wrote a big chunk of the current documentation and probably understand the system enough to help me with my first trivial use case.

[Susurrus]

The docs also refer to a bad variant within ioctl, which doesn't appear to exist. This should shortcut the various _IOC and related macros and use the hardcoded number instead. This is necessary for use with the TIOCEXCL and many other constants.

[asomers]

@cmr it looks like you originally wrote the macros. Could you please add some documentation?

[roblabla]

I spent some time using the ioctl module, so I have a pretty good understanding of it.

First, the ioc!, io!, ior!, iow! and iorw! macros correspond to the equivalent macros _IOC, _IO, ... defined by the linux kernel there. They allow to calculate, given a direction, an ioctl "type", an ioctl number, and the size of the ioctl argument, an unique identifier for that ioctl.

The ioc_dir, ioc_nr, ioc_size and ioc_type function allow to reverse what the above macro do. Basically, ioc_dir(io!(0, 0)) == ioctl::NONE (no direction, because I used io!). Those functions aren't very useful AFAIK. I haven't had to use them anyhow.

The ioctl function allows sending an ioctl given an fd: RawFd, request_number: c_ulong, .... It's hidden from the docs with a mention that it should be removed in favor of the libc function when rust-lang/rust#26809 is merged. Which was merged a while ago. So maybe it's time for this function to retire ?

Finally, the ioctl! macro defines an unsafe function that basically looks like this (minus some differences depending on the variant of ioctl used) :

// Result of ioctl!(read ioctl_name with b'b', 1; Type)
pub unsafe fn ioctl_name(fd: libc::c_int, val: *mut Type)
                            -> nix::Result<libc::c_int> {
    // Note that ioctl function used here should be libc::ioctl. The pr mentioned a
    // https://github.com/nix-rust/nix/blob/master/src/sys/ioctl/mod.rs#L117 was merged
    Errno::result(nix::sys::ioctl::ioctl(fd, ior!(b'b', 1,size_of::<Type>()) as libc::c_ulong, val))
}

There are multiple variants that depend on the ioctl definition :

• ioctl!(none ioctl_name with b'b', 1) => An ioctl that takes no argument. It will use io! to calculate the request number.
• ioctl!(read ioctl_name with b'b', 1; u8) => An ioctl that takes a *mut u8 (it will return a value at the given pointer). It will use ior!.
• ioctl!(write ioctl_name with b'b', 1; u8) => An ioctl that takes a *const u8 (it takes the value at the given pointer). It will use iow!
• ioctl!(readwrite ioctl_name with b'b', 1; u8) => An ioctl that takes a *mut u8 (it will both read the value at the given pointer, and modify it). It will use iorw!.
• ioctl!(read buf ioctl_name with b'b', 1; u8) => An ioctl that returns an array of u8. It will take a *mut u8, and a len: size_t.
• ioctl!(write buf ioctl_name with b'b', 1; u8) => same, but takes an array instead of returning it
• ioctl!(readwrite buf ioctl_name with b'b', 1; u8) => same, but both takes an array and modify it
• ioctl!(ioctl_name with 0x23) => This is what a bad ioctl is. It basically just creates an alias to the ioctl function.

I guess there's a mistake in the documentation concerning this.

I think a good plan would be this :

• Make all the macro except ioctl! hidden from the docs. I don't think it's possible to prevent exporting them, due to how the macro system works, but the users shouldn't use them directly. They're basically an implementation detail.
• convert_ioctl_res should be removed. It doesn't serve any purpose as far as I can see, and clutters the docs with one more macro.
• The docs should be rectified concerning the bad ioctl.
• The docs should explain a bit better that ioctl generates functions.
• https://docs.rs/nix/0.8.1/nix/macro.ioctl.html should have a link to the ioctl module at least, and some (maybe more basic) docs as well.
• Similar to error_chain, it'd be nice to have examples of the generated functions. See : https://docs.rs/error-chain/0.10.0/error_chain/example_generated/index.html
• I'm not sure what to do for the ioc_dir family of function. I don't think they serve much purpose, but I could be wrong.

PS: the read buf family of the ioctl! macro seems weird to me, I've never seen an ioctl that would actually need it. Anyone got an example of it ? Its implementation also seems a bit broken to me, shouldn't it use len * size_of::<$ty> instead of just len ?

[emberian]

Make all the macro except ioctl! hidden from the docs. I don't think it's possible to prevent exporting them, due to how the macro system works, but the users shouldn't use them directly. They're basically an implementation detail.

They aren't just an implementation detail. The ioctl macro can't handle a few of the truly obscure ioctl "protocols".

I don't know about "read buf". I'm willing to bet it's buggy, I haven't had to use it. At one point I scraped and auto-generated https://github.com/cmr/ioctl/blob/master/src/platform/linux-generated-x86_64.rs from the Linux headers.

The original idea with my ioctl crate was to have a one-stop shop, but when I went to include it in nix they didn't want the system-specific concrete ioctls... The idea now is to wrap the ioctls you need ad hoc in crates which do something. Eg, my evdev crate wraps all the evdev ioctls.

[emberian]

(Sorry for slow response, busy at OPLSS)

[Susurrus]

One example that isn't addressed here is for ioctls that have no arguments. For example, I want to do something like libc::ioctl(fd, TIOCEXCL). It doesn't appear that the ioctl! macro supports this use case. Neither does the module appear to re-export libc::ioctl. Is that correct? Do you think this is a valid use-case that should be supported by the macro, such that that's the only wrapper nix provides for writing ioctl APIs?

[roblabla]

Is there value in providing such a wrapper ?

Technically speaking, the module does export libc::ioctl (well, as noted above, it's not directly libc::ioctl, but it should be), but it hides it from the doc see here. It probably shouldn't.

[Susurrus]

@roblabla What do you mean by that comment? You mean we should re-export libc::ioctl in addition to our own ioctl! macro?

[Susurrus]

My use case for TIOCEXCL is still not supported by nix if I understand this discussion. Basically bad still assumes that there's data, so should a bad_none also be added?

[Susurrus]

@roblabla If you look at the rust-spidev source you'll see that the * buf ioctls expect the user to pass the total size in bytes as len, not just the number of elements. I think we should move that into our macro-generated function instead of relying on the user to do that. But at that point why even have these consume a raw pointer when they could take a slice reference and pull the length straight from that? I imagine there's a safety issue with that, but I'm not seeing it.

[Susurrus]

I'm also trying to figure out if the generated ioctl! can't be made safe by default if we use & and &mut references instead of raw pointers. I would think this should work, so I tacked on a commit that changes all of them. Please check out my latest changes to #670. I'd love to get some feedback there.

[Susurrus]

Also, if you wanted to see what the API looks like for a new project, I've added a branch for replacing my ioctl-rs dependency with using nix's ioctl! macro directly instead. You can see what this looks like here.

[posborne]

I'm also trying to figure out if the generated ioctl! can't be made safe by default if we use & and &mut references instead of raw pointers. I would think this should work, so I tacked on a commit that changes all of them. Please check out my latest changes to #670. I'd love to get some feedback there.

I don't think we can ever correctly make these generated functions safe. In order to make the calls safe many system calls require additional semantics (that go well beyond the ioctl call itself) be followed. A common pattern for libraries will be to use the macros to get the FFI bindings in a convenient, declarative way and then provide safe wrappers around those.

[Susurrus]

This leads me to ask, should these functions take references then or are raw pointers still better semantics? And if we're not providing a safe API here, why is this in nix?

[roblabla]

References can't be null. Some ioctl might need this ?

…

On July 13, 2017 10:43:48 PM GMT+02:00, Susurrus ***@***.***> wrote: This leads me to ask, should these functions take references then or are raw pointers still better semantics? And if we're not providing a safe API here, why is this in `nix`? -- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: #641 (comment)

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

[Susurrus]

@roblabla That's a good point, I wonder if there are any ioctls that actually do that. Do you know of any other Rust projects using ioctl besides rust-spidev?

I think it's better that I keep the raw pointers for now in #670. I think after that this should be good to review. Would you have some time soon to look it over @roblabla?

[roblabla]

Sorry, had an extended week-end. I'll take a look at #670 :)