Add additional <termios.h> mappings #1026
Conversation
|
Unfortunately mappings need to go in two places -- where you made the change, and in Could you also show a full example (including reduced source code) in the commit message, so |
fanquake
force-pushed
the
add_more_termios_mappings
branch
from
d4308df to
14537b6
Compare
|
Thanks. Apologies for the slow follow up. I've added the mappings to |
|
Thank you! The CI failed due to an upstream Clang change, so I updated that in #1027. Please rebase once that's merged, so we can see that CI runs green before merge here. I'll merge that within half an hour. |
```cpp
include <bits/termios-c_lflag.h> // for ECHO
include <bits/termios-struct.h> // for termios
include <bits/termios-tcflow.h> // for TCSANOW
void SetStdinEcho(bool enable)
{
struct termios tty;
tcgetattr(STDIN_FILENO, &tty);
if (!enable) {
tty.c_lflag &= ~ECHO;
} else {
tty.c_lflag |= ECHO;
}
(void)tcsetattr(STDIN_FILENO, TCSANOW, &tty);
}
```
fanquake
force-pushed
the
add_more_termios_mappings
branch
from
14537b6 to
2e34f85
Compare
No worries. Have rebased on master. |
|
Very nice, thank you! |
|
Hi, I'm not sure about the correctness of this change. Please check some relevant threads in the linux-man@ mailing list: Maybe @pali can help. |
|
@alejandro-colomar Is that discussion for kernel-side |
It affects user space, AFAIR. |
|
Hello! There are two incompatible and different definitions of termios macros. One is in So, if you are using libc functions like This mess means that you cannot use both |
|
@pali Thanks for the additional info! So it seems to me this mapping from |
Yes, this PR seems safe.
Not sure. The IOCTL IDs come from |
9b0a13a tidy: Add include-what-you-use (fanquake) 74cd038 refactor: fix includes in src/init (fanquake) c79ad93 refactor: fix includes in src/compat (fanquake) Pull request description: We recently added a [`clang-tidy` job](https://github.com/bitcoin/bitcoin/blob/master/ci/test/00_setup_env_native_tidy.sh) to the CI, which generates a compilation database. We can leverage that now existing database to begin running [include-what-you-use](https://include-what-you-use.org/) over the codebase. This PR demonstrates using a mapping_file to indicate fixups / includes that may differ from IWYU suggestions. In this case, I've added some fixups for glibc includes that I've [upstreamed changes for](include-what-you-use/include-what-you-use#1026): ```bash # Fixups / upstreamed changes [ { include: [ "<bits/termios-c_lflag.h>", private, "<termios.h>", public ] }, { include: [ "<bits/termios-struct.h>", private, "<termios.h>", public ] }, { include: [ "<bits/termios-tcflow.h>", private, "<termios.h>", public ] }, ] ``` The include "fixing" commits of this PR: * Adds missing includes. * Swaps C headers for their C++ counterparts. * Removes the pointless / unmaintainable `//for abc, xyz` comments. When using IWYU, if anyone wants to see / generate those comments, to see why something is included, it is trivial to do so (IWYU outputs them by default). i.e: ```cpp // The full include-list for compat/stdin.cpp: #include <compat/stdin.h> #include <poll.h> // for poll, pollfd, POLLIN #include <termios.h> // for tcgetattr, tcsetattr #include <unistd.h> // for isatty, STDIN_FILENO ``` TODO: - [ ] Qt mapping_file. There is one in the IWYU repo, but it's for Qt 5.11. Needs testing. - [ ] Boost mapping_file. There is one in the IWYU repo, but it's for Boost 1.75. Needs testing. I'm not suggesting we turn this on the for entire codebase, or immediately go-nuts refactoring all includes. However I think our dependency includes are now slim enough, and our CI infrastructure in place such that we can start doing this in some capacity, and just automate away include fixups / refactorings etc. ACKs for top commit: MarcoFalke: review ACK 9b0a13a jonatack: ACK 9b0a13a reviewed changes and run CI output in https://cirrus-ci.com/task/4750910332076032 Tree-SHA512: 00beab5a5f2a6fc179abf08321a15391ecccaa91ab56f3c50c511e7b29a0d7c95d8bb43eac2c31489711086f6f77319d43d803cf8ea458e7cd234a780d9ae69e
a395218 ci, iwyu: Drop backported mappings (Hennadii Stepanov) Pull request description: See include-what-you-use/include-what-you-use#1026. Split from #27710 as a non-controversial change. ACKs for top commit: fanquake: ACK a395218 Tree-SHA512: ae7955a99396ab4f62ab7c989dba59c26448837f0ba4436bf3fddebe4099b5d3c03492e22a8497104c6afcceede1bb1b81f9d71c7c7e43692e6d70dcdfc11e7c
a395218 ci, iwyu: Drop backported mappings (Hennadii Stepanov) Pull request description: See include-what-you-use/include-what-you-use#1026. Split from bitcoin#27710 as a non-controversial change. ACKs for top commit: fanquake: ACK a395218 Tree-SHA512: ae7955a99396ab4f62ab7c989dba59c26448837f0ba4436bf3fddebe4099b5d3c03492e22a8497104c6afcceede1bb1b81f9d71c7c7e43692e6d70dcdfc11e7c
a395218 ci, iwyu: Drop backported mappings (Hennadii Stepanov) Pull request description: See include-what-you-use/include-what-you-use#1026. Split from bitcoin#27710 as a non-controversial change. ACKs for top commit: fanquake: ACK a395218 Tree-SHA512: ae7955a99396ab4f62ab7c989dba59c26448837f0ba4436bf3fddebe4099b5d3c03492e22a8497104c6afcceede1bb1b81f9d71c7c7e43692e6d70dcdfc11e7c
4 participants
Uh oh!
There was an error while loading. Please reload this page.