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

Allow varargs for libc::open when it is allowed by the second argument #1970

Merged
merged 1 commit into from
Mar 5, 2022

Conversation

asquared31415
Copy link
Contributor

This PR allows libc::open to be called using two or three arguments as defined in https://man7.org/linux/man-pages/man2/open.2.html

The presence of the third argument depends on the value of the second argument. If the second argument dictates that the third argument is required miri will emit an error if the argument is missing. If the second argument does not require a third argument, then the argument is ignored and passed as 0 internally (it would be ignored by libc anyway)

@asquared31415
Copy link
Contributor Author

One concern I have is with the tests, is #[cfg(unix)] the best way to do these tests and is that cfg correct for testing this specifically?

@RalfJung
Copy link
Member

RalfJung commented Feb 6, 2022

Thanks a lot for the PR!

Just as a heads-up, I am busy with job interviews for probably at least another month, so it will be a while until I will be able to review this PR.

src/shims/posix/fs.rs Outdated Show resolved Hide resolved
src/shims/posix/fs.rs Outdated Show resolved Hide resolved
@RalfJung
Copy link
Member

RalfJung commented Mar 5, 2022

Thanks. :)
@bors r+

@bors
Copy link
Contributor

bors commented Mar 5, 2022

📌 Commit 8e97599 has been approved by RalfJung

@bors
Copy link
Contributor

bors commented Mar 5, 2022

⌛ Testing commit 8e97599 with merge 3854a76...

@bors
Copy link
Contributor

bors commented Mar 5, 2022

☀️ Test successful - checks-actions
Approved by: RalfJung
Pushing 3854a76 to master...

@bors bors merged commit 3854a76 into rust-lang:master Mar 5, 2022
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