Move more ioctl constants and flags to linux::arch::* modules. #2530
Conversation
|
r? @JohnTitor (rust-highfive has picked a reviewer for you, use r? to override) |
de-vri-es
changed the title
de-vri-es
force-pushed
the
tty-cleanup
branch
2 times, most recently
from
9f88c82
to
27c9174
Compare
de-vri-es
force-pushed
the
tty-cleanup
branch
from
27c9174
to
3fa2809
Compare
|
@bors r+ |
|
📌 Commit 3fa2809 has been approved by |
|
☀️ Test successful - checks-actions, checks-cirrus-freebsd-11, checks-cirrus-freebsd-12, checks-cirrus-freebsd-13 |
|
@de-vri-es According to https://github.com/rust-lang/libc/runs/4218181867?check_suite_focus=true, this changed some consts' type, it's a breaking change. If that change is necessary, I'd like to deprecate them first then do the actual change in favor of https://github.com/rust-lang/libc/blob/master/CONTRIBUTING.md#breaking-change-policy, could you revert the breaking change part? |
|
Ah I didn't notice that aarch64-musl has different types for the ioctl constants compared to the glibc targets D: However, reverting that would mean putting everything back in libc+arch specific modules. Or alternatively, I could add a What do you think is best? |
|
Sounds fine, one problem is that older Rust doesn't support |
|
Alright, sounds good. I think I'll have time for that tomorrow. If this is blocking a release it might be best to simply revert the whole PR until I can put the fix in. |
|
Fix proposed in #2550 |
Use libc specific type for architecture specific ioctl defines on Linux. This PR should fix the type change of some ioctl constants on Linux introduced by #2530. It does this by adding a `#[doc(hidden)]` type called `Ioctl`, which is defined in libc specific modules and used in arch specific modules. However, when doing this I noticed that when I added `TCGETS2`, `TCSETS2`, ... in #2508, I unconditionally used `c_ulong`. This is inconsistent with the other ioctl constants for `musl` and `uclibc`. This PR also changes those to use the libc specific types. However, PR #2508 has already been released in 0.2.107, so technically that is also a semver incompatible change. The impact is limited to new constants introduced in the last release, and only on `musl` and `uclibc` targets. So what is more important here? Consistency in the type of ioctl constants, or being very strict with backwards compatibility? If it is the latter, I'll revert `TCGETS2` etc to `c_ulong` for this PR.
This PR moves a few things to
linux::arch::*modules and out of libc specific modules. Specifically, it:TIOCM*ioclt constants and associated flags.BOTHERflag.IBSHIFTflag.Without this PR, there are some arch+libc combinations that are missing some of these constants. Since these are all ioctl numbers or arguments to ioctls, they are not specific to any libc flavour.
BOTHERandIBSHIFTdon't even appear in glibc or musl headers.