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

Remove unsafe and unnecessary size argument from FileDesc::read() #821

Merged
merged 1 commit into from
May 3, 2024

Conversation

martinvonz
Copy link
Contributor

@martinvonz martinvonz commented Sep 18, 2023

The size argument to FileDesc::read() is not checked against the length of the buffer, so libc::read() could end up writing past the buffer if we passed a size that's too large. However, we always pass exactly the size of the buffer, so that doesn't happen. Let's just remove the argument since it's not currently needed, thereby removing the risk of bugs if the function is used incorrectly by future callers.

This came up in review of unsafe Rust code at my company.

The `size` argument to `FileDesc::read()` is not checked against the
length of the buffer, so `libc::read()` could end up writing past the
buffer if we passed a size that's too large. However, we always pass
exactly the size of the buffer, so that doesn't happen. Let's just
remove the argument since it's not currently needed, thereby removing
the risk of bugs if the function is used incorrectly by future
callers.

This came up in review of `unsafe` Rust code at my company.
@Shnatsel
Copy link

Are there any blockers to merging this? Is there any way I can help expedite this PR?

@TimonPost TimonPost merged commit 6fde554 into crossterm-rs:master May 3, 2024
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.

3 participants