Add ioctl generation macros for linux

[sm-Fifteen]

The _IOC, _IO, _IOR, _IOW and _IOWR macros, which are exposed by both musl (which redefines them per architecture) and glibc (which includes them from the linux headers) don't currently have equivalents in Rust's libc. Given there are a lot of ioctls out there and that the format is known to be architecture-specific, not having access to those macros to generate ioctl request numbers means having to duplicate that platform-specific logic elsewhere like the nix crate currently does. Moving these into the libc crate alongside all of the other musl/glibc would (as far as I can tell) be more in line with the goals of this crate.

With that said, I'm currently having trouble figuring out how those should be tested. Most of the ioctl codes currently defined in libc follow the short 16 bits format, and the few of them using the longer format are architecture-specific redefinitions, like TIOCGPGRP which is is defined as 0x540F in asm-generic but as _IOR('t', 119, int) for mips and powerpc and as _IOR('t', 131, int) for sparc, so testing by ensuring that the manual ioctl definitions match the macro's output most likely wouldn't work. We could always use a list of driver-specific IOCTLs with known type letters and sequence numbers and testing that those match with the kernel headers ones, but I'm not familiar enough with the way the tests are setup to get that to work.

Another thing I'm not sure how to do is declaring those as const_fn, especially since I would need size_of to be const_fn as well for that to work. I know there is a libc_const_size_of config switch for that and I have noticed that macros are available for this as well, but the macros are undocumented and thier usage is unclear, so I've left those as plain functions for now.

The reason why the definitions don't follow the same format between musl and glibc is that I was attempting to mimic how musl defines its own macros per architecture while glibc imports them from the kernel header files where platform-specific constants tend to be defined in the platform-specific headers which then re-use the generic macro. I'm not entirely sure whether this is preferable to sticking to one way or the other.

I'm open to suggestions on what the best way of handling this is, or if someone thinks this may not actually belong in libc.

[rust-highfive]

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.

[sm-Fifteen]

I think I'm doing something wrong with musl, I'm getting syntax errors from the powerpc module (whoops) in the CI results for other architectures. Could that have something to do with the f! macro?

---

EDIT: Ok, so it does seem to be the case, and the f! macro also solved the const-ness issue, which is nice, though it looks like it doesn't like me using generics to handle the type argument:

   Compiling libc v0.2.66 (/home/user/git_other/libc)
error: no rules expected the token `<`
   --> src/unix/linux_like/linux/gnu/mod.rs:982:24
    |
982 |     pub {const} fn _IOW<T: Sized> (a:u8,b:u8) -> ::c_ulong {
    |                        ^ no rules expected this token in macro call
    | 
   ::: src/macros.rs:185:9
    |
185 |         macro_rules! f {
    |         -------------- when calling this macro

I figured a crate like libc might not be the right place to use generics like that, though I'm not entirely sure how I'm supposed to feed _IOW, _IOR and _IOWR with the size of a type without that...

[bors]

☔ The latest upstream changes (presumably #1899) made this pull request unmergeable. Please resolve the merge conflicts.

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

[workingjubilee]

It's a pity this languished because it would make the libc code more structured and significantly help with musl support, which has recently been getting more attention (including from er, me, I guess). Needs a rebase though at this point.

[sm-Fifteen]

It's a pity this languished because it would make the libc code more structured and significantly help with musl support, which has recently been getting more attention (including from er, me, I guess).

Yeah, not having support in the lower level libraries for ioctls of all things is very strange to me, given it's one of the basic file descriptor interactions you can have on Unix platforms. Not having them in libc (which otherwise has all the platform-specific code by design, that's kind of what it's here for) means it has to be implemented and maintained at a higher level, like the nix crate or use a bit of rust-bindgen for the ioctl codes and hand-rolled macros for file descriptor interaction, like with Mozilla's own CTAP HID module.

If there's interest, I can try rebasing the conflicting GNU libc files so this is reviewable and mergable.

[workingjubilee]

Please do! I can't r+ things myself (over here, I mean) but I have a stick, I have an idea of what I'm reviewing, and I can poke people with it who can. :^)

[joshtriplett]

👍 for including these in libc.

I do think they want to be const fn if at all possible.

[JohnTitor]

r? @joshtriplett

@sm-Fifteen Could you rebase onto master or open a new PR if you're still interested in this change?

[sm-Fifteen]

@sm-Fifteen Could you rebase onto master or open a new PR if you're still interested in this change?

@JohnTitor: Right, I'll get to that ASAP. Should I also write some unit tests to make sure that the values generated here match what the respective C header macros generate? It's my first PR here, and given how this is unsafe-adjacent code, that makes me a bit nervous, especially for the architectures like sparc and ppc, where the ioctl structure diverges from what's used for x86.

I do think they want to be const fn if at all possible.

@joshtriplett: Is it ok by now to mark these as const fn directly? Making all of these const was my intention initially, but I couldn't figure out how to get the "conditional const where supported" macros to work.

[JohnTitor]

Should I also write some unit tests to make sure that the values generated here match what the respective C header macros generate?

I guess it should? How to add unit tests, see https://github.com/rust-lang/libc/blob/master/libc-test/test/makedev.rs for example.

Is it ok by now to mark these as const fn directly? Making all of these const was my intention initially, but I couldn't figure out how to get the "conditional const where supported" macros to work.

Yes, it's totally fine, it's conditional by default:

libc/build.rs

Lines 100 to 113 in c20064f

	// Rust >= 1.62.0 allows to use `const_extern_fn` for "Rust" and "C".
	if rustc_minor_ver >= 62 {
	println!("cargo:rustc-cfg=libc_const_extern_fn");
	} else {
	// Rust < 1.62.0 requires a crate feature and feature gate.
	if const_extern_fn_cargo_feature {
	if !is_nightly || rustc_minor_ver < 40 {
	panic!("const-extern-fn requires a nightly compiler >= 1.40");
	}
	println!("cargo:rustc-cfg=libc_const_extern_fn_unstable");
	println!("cargo:rustc-cfg=libc_const_extern_fn");
	}
	}
	}

[JohnTitor]

Closing as inactive, feel free to reopen/resubmit if you're still interested in this change. Thank you!

[habnabit]

if this is revived.. could those be const fn _IO* ?

[sm-Fifteen]

if this is revived.. could those be const fn _IO* ?

I'm honestly not too sure anymore whether ioctls should belong in there or in libc anymore, but I believe the nix crate has since gained this feature (with support for all the architectures defined in the Linux uapi), if you need something you can use right now. If the parameters can all be resolved into a constant value at compile time, it looks like that's what the macro would do for the wrapper function (it wraps the ioctl(fd) call as well) that it creates.

[habnabit]

if this is revived.. could those be const fn _IO* ?

I'm honestly not too sure anymore whether ioctls should belong in there or in libc anymore, but I believe the nix crate has since gained this feature (with support for all the architectures defined in the Linux uapi), if you need something you can use right now. If the parameters can all be resolved into a constant value at compile time, it looks like that's what the macro would do for the wrapper function (it wraps the ioctl(fd) call as well) that it creates.

i'd use whatever for raymanfx/libv4l-rs#118 if nix is a better option. i used rustix after i saw this PR wasn't merged.