-
Notifications
You must be signed in to change notification settings - Fork 336
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
Add syscall dup()
for unix target
#3707
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR!
src/shims/unix/fd.rs
Outdated
match this.machine.fds.dup(old_fd) { | ||
Some(dup_fd) => { | ||
if new_fd != old_fd { | ||
// Close new_fs if it is previously opened. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// Close new_fs if it is previously opened. | |
// Close new_fs if it is previously opened. | |
// (If old == new, then `dup_fd` ensure we keep the underlying file description alive.) |
src/shims/unix/fd.rs
Outdated
// Close new_fs if it is previously opened. | ||
if let Some(file_descriptor) = this.machine.fds.remove(new_fd) { | ||
// Ignore close error according to dup2() doc. | ||
file_descriptor.close(this.machine.communicate()).ok(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nono, we can never ignore this. This could ignore UB!
You can ignore the Scalar
, but the ?
that propagates interpreter errors must be present.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Totally. I didn't distinguish interpreter's error from program's error back then.
src/shims/unix/fd.rs
Outdated
file_descriptor.close(this.machine.communicate()).ok(); | ||
} | ||
|
||
this.machine.fds.fds.insert(new_fd, dup_fd); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
insert
returns the old value if any, so instead of calling remove
first you can just use that.
Looks good, thanks a lot :) |
☀️ Test successful - checks-actions |
Add support for
dup()
anddup2()
.Fixes #3454