Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use from_bits_retain for all Termios wrapper fields. #2253

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

qwandor
Copy link
Contributor

@qwandor qwandor commented Dec 7, 2023

What does this PR do

In case there are any unrecognised bits, they should be kept when setting fields of the underlying libc termios struct in get_libc_termios, rather than being dropped. This ensures that termios can be roundtripped even with unrecognised bits set.

(I came across this issue on Android in part of the NFC stack, but it should also fix #2071.)

Checklist:

  • I have read CONTRIBUTING.md
  • I have written necessary tests and rustdoc comments
  • A change log has been added if this PR modifies nix's API

In case there are any unrecognised bits, they should be kept when
setting fields of the underlying libc termios struct in
get_libc_termios, rather than being dropped. This ensures that termios
can be roundtripped even with unrecognised bits set.
Copy link
Member

@asomers asomers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this PR fix #2071 ?

@qwandor
Copy link
Contributor Author

qwandor commented Dec 8, 2023

Would this PR fix #2071 ?

I think so, can you test it?

@asomers
Copy link
Member

asomers commented Dec 8, 2023

Would this PR fix #2071 ?

I think so, can you test it?

No, I can't. I don't have an Illumos system. Maybe @nospam3089 can test it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

On illumos tcsetattr() is unusably buggy as it truncates termios fields
2 participants