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

Termios needs a safe constructor #1115

Closed
asomers opened this issue Aug 31, 2019 · 5 comments · Fixed by #1235
Closed

Termios needs a safe constructor #1115

asomers opened this issue Aug 31, 2019 · 5 comments · Fixed by #1235
Assignees

Comments

@asomers
Copy link
Member

asomers commented Aug 31, 2019

The only constructor method for Termios is the unsafe and undocumented default_uninit. All of the examples use that function. But not only is it unsafe, it also uses std::mem::uninitialized, which will be deprecated in Rust 1.38.0. This will soon be the last remaining use of std::mem::uninitialized.

asomers added a commit to asomers/nix that referenced this issue Dec 1, 2019
Replace it with mem::zeroed.  It isn't perfect, but it's better than it
was.

Issue nix-rust#1115
bors bot added a commit that referenced this issue Dec 1, 2019
1158: Remove the last use of mem::uninitialized r=asomers a=asomers

Replace it with mem::zeroed.  It isn't perfect, but it's better than it
was.

Issue #1115

Co-authored-by: Alan Somers <asomers@gmail.com>
kevinwern pushed a commit to kevinwern/nix that referenced this issue Dec 2, 2019
Replace it with mem::zeroed.  It isn't perfect, but it's better than it
was.

Issue nix-rust#1115
@asomers
Copy link
Member Author

asomers commented Apr 19, 2020

ping @Susurrus would you have some time to try fixing this?

@Susurrus
Copy link
Contributor

Susurrus commented May 7, 2020

Is there anything to fix? #1158 got rid of mem::uninitialized(), and zeroing it seems fine. If anything switching it from doc(hidden) to pub(crate) seems more useful (same with the below update_wrapper() function.

@asomers
Copy link
Member Author

asomers commented May 8, 2020

Is all zeros actually a valid state for a libc::termios? If so, then zeroing it is ok. But if not then we should just delete that function and force people to use proper constructors like tcgetattr. Also, we should remove default_uninit from the API docs.

@Susurrus
Copy link
Contributor

Susurrus commented May 8, 2020

This is effectively a private function because of the doc(hidden) (which was the way to do this before pub(crate)). That's why I suggested changing it to pub(crate) and being done (however this is technically a breaking change). I don't use it in serialport, which would likely be one of the only users, so I imagine it won't impact users much.

@asomers
Copy link
Member Author

asomers commented May 8, 2020

Yeah, I agree with making it pub(crate). But even though it's currently #[doc(hidden)], it's still used in the API docs (in hidden form), so you'll have to replace it with something or the doc tests will break.

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 a pull request may close this issue.

2 participants