Add TCGETS2 and TCSETS2 (and variants) ioctl constants for Linux. #2508
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
r? @Amanieu (rust-highfive has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
de-vri-es
force-pushed
the
tcgets2-and-friends
branch
4 times, most recently
from
05b4bdf
to
75b7725
Compare
Amanieu
reviewed
Nov 4, 2021
Amanieu
reviewed
Nov 4, 2021
de-vri-es
force-pushed
the
tcgets2-and-friends
branch
from
75b7725
to
1338e2e
Compare
|
@bors r+ |
|
📌 Commit 1338e2e has been approved by |
de-vri-es
force-pushed
the
tcgets2-and-friends
branch
from
1338e2e
to
8c9bfe0
Compare
Oops, I pushed a new commit to fix |
|
@bors r+ |
|
📌 Commit 8c9bfe0 has been approved by |
bors
added a commit
that referenced
this pull request
Nov 4, 2021
Add TCGETS2 and TCSETS2 (and variants) ioctl constants for Linux. I noticed that the `termios2` struct is already exposed, but the ioctl constants to use it are not. This PR adds the `TCGETS2`, `TCSETS2`, `TCSETSW2` and `TCSETSF2` on Linux so that you can actually do something with the `termios2` struct. The `powerpc` architecture is notably missing, because it does not seem to support the `TCGETS2`/`TCSETS2` ioctls. I *think* the constants are correct for all platforms, but I'm also not 100% sure. Do the unit tests verify the values for all supported platforms, by any chance?
|
💔 Test failed - checks-actions |
de-vri-es
force-pushed
the
tcgets2-and-friends
branch
from
8c9bfe0
to
447e868
Compare
|
Ugh, the Yay for the test suite though <3 |
|
Yea the tests here are a lifesaver. @bors r+ |
|
📌 Commit 447e868 has been approved by |
bors
added a commit
that referenced
this pull request
Nov 5, 2021
Add TCGETS2 and TCSETS2 (and variants) ioctl constants for Linux. I noticed that the `termios2` struct is already exposed, but the ioctl constants to use it are not. This PR adds the `TCGETS2`, `TCSETS2`, `TCSETSW2` and `TCSETSF2` on Linux so that you can actually do something with the `termios2` struct. The `powerpc` architecture is notably missing, because it does not seem to support the `TCGETS2`/`TCSETS2` ioctls. I *think* the constants are correct for all platforms, but I'm also not 100% sure. Do the unit tests verify the values for all supported platforms, by any chance?
|
💔 Test failed - checks-actions |
de-vri-es
force-pushed
the
tcgets2-and-friends
branch
from
447e868
to
fc89d7c
Compare
|
I had a typo in TCSETSF2 for sparc 😢 Fixed it now. Also, I noticed that the constants are all Or I can just leave them as |
Amanieu
reviewed
Nov 6, 2021
de-vri-es
force-pushed
the
tcgets2-and-friends
branch
from
fc89d7c
to
58a015c
Compare
|
@bors r+ |
|
📌 Commit 58a015c has been approved by |
|
☀️ Test successful - checks-actions, checks-cirrus-freebsd-11, checks-cirrus-freebsd-12, checks-cirrus-freebsd-13 |
|
\o/ Thanks for the reviews! |
bors
added a commit
that referenced
this pull request
Nov 6, 2021
Move termios2 struct and ioctl constants to linux::arch::*. When trying to use the newly added ioctls from #2508 I noticed that the `termios2` struct was not defined for musl or uclibc, except interestingly for the hexagon+musl target However, the `termios2` struct and ioctls are part of the Linux headers, not part of the libc headers. So the libc flavour doesn't matter for these definitions. This PR moves the definitions to the `linux::arch::*` modules, to make them available for all libc flavours and reduce code duplication as a bonus.
bors
added a commit
that referenced
this pull request
Nov 6, 2021
Move termios2 struct and ioctl constants to linux::arch::*. When trying to use the newly added ioctls from #2508 I noticed that the `termios2` struct was not defined for musl or uclibc, except interestingly for the hexagon+musl target However, the `termios2` struct and ioctls are part of the Linux headers, not part of the libc headers. So the libc flavour doesn't matter for these definitions. This PR moves the definitions to the `linux::arch::*` modules, to make them available for all libc flavours and reduce code duplication as a bonus.
bors
added a commit
that referenced
this pull request
Nov 18, 2021
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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I noticed that the
termios2struct is already exposed, but the ioctl constants to use it are not. This PR adds theTCGETS2,TCSETS2,TCSETSW2andTCSETSF2on Linux so that you can actually do something with thetermios2struct.The
powerpcarchitecture is notably missing, because it does not seem to support theTCGETS2/TCSETS2ioctls.I think the constants are correct for all platforms, but I'm also not 100% sure. Do the unit tests verify the values for all supported platforms, by any chance?