Skip to content

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

Merged
bors merged 1 commit into
rust-lang:masterfrom
asquared31415:open_unix_varargs
Mar 5, 2022
Merged

Allow varargs for libc::open when it is allowed by the second argument#1970
bors merged 1 commit into
rust-lang:masterfrom
asquared31415:open_unix_varargs

Conversation

@asquared31415

Copy link
Copy Markdown
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
Copy Markdown
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

RalfJung commented Feb 6, 2022

Copy link
Copy Markdown
Member

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.

Comment thread src/shims/posix/foreign_items.rs Outdated
Comment thread src/shims/posix/fs.rs Outdated
Comment thread tests/compile-fail/fs/unix_open_missing_required_mode.rs Outdated
Comment thread src/shims/posix/fs.rs Outdated
@RalfJung

RalfJung commented Mar 5, 2022

Copy link
Copy Markdown
Member

Thanks. :)
@bors r+

@bors

bors commented Mar 5, 2022

Copy link
Copy Markdown
Contributor

📌 Commit 8e97599 has been approved by RalfJung

@bors

bors commented Mar 5, 2022

Copy link
Copy Markdown
Contributor

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

@bors

bors commented Mar 5, 2022

Copy link
Copy Markdown
Contributor

☀️ 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