Skip to content

Support more file operations on stdin/stdout/stderr #1499

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

Open
2 of 5 tasks
RalfJung opened this issue Aug 5, 2020 · 6 comments
Open
2 of 5 tasks

Support more file operations on stdin/stdout/stderr #1499

RalfJung opened this issue Aug 5, 2020 · 6 comments
Labels
A-shims Area: This affects the external function shims C-enhancement Category: a PR with an enhancement or an issue tracking an accepted enhancement

Comments

@RalfJung
Copy link
Member

RalfJung commented Aug 5, 2020

Thanks to recent work by @samrat, the standard FDs 0-2 (stdin/stdout/stderr, but I never know in which order) are now sharing the code with the FDs from opened files. However, for some operations we fall back to working on FileHandle directly.

It should be possible to stop doing that, and support all operations also on FDs 0-2. In particular, I think it would be great to support "dup" to let a program put some file as stdout, or stdin.

In the end, all uses of as_file_handle should be gone, and everything work through the trait:

  • "dup"
  • "fullfsync", "fsync", "fdatasync", "sync_file_range" (probably NOPs on stdin and errors for the others, I don't think we can flush in a cross-platform way)
  • "close"
  • "ftruncate64"... well this probably can just error
  • Move FileMetadata::from_fd to the trait as well
@RalfJung RalfJung added A-shims Area: This affects the external function shims C-enhancement Category: a PR with an enhancement or an issue tracking an accepted enhancement labels Aug 5, 2020
@RalfJung
Copy link
Member Author

@samrat if you are looking for more ways to contribute, this seems like an obvious next step. :)

@samrat
Copy link
Contributor

samrat commented Aug 14, 2020

@RalfJung Yes! I will take this up :)

@RalfJung
Copy link
Member Author

Awesome. :) As usual, let us know if you need any help or have any questions.

@RalfJung
Copy link
Member Author

My thinking is that we should move all operations to the trait so that FileHandle becomes a proper abstraction that is not used outside the trait. But do y'all think that this is worth it?
Cc @oli-obk

@oli-obk
Copy link
Contributor

oli-obk commented Aug 17, 2020

Hmm... "worth it" sounds like there are downsides to this? The complexitiy in #1511 seems ok to me

@RalfJung
Copy link
Member Author

Well, the alternative is to keep using as_file_handle as we do now, which has the advantage of already being implemented. ;)

We have more operations that the std* handles do not support than I anticipated. I am not sure whether it is easier to just have a single method in the trait to reflect this (as_file_handle, with the actual handling code then being at the respective shim) or one method per operation (with the handling code being separated from the code to glue this to the rest of Miri).

bors added a commit that referenced this issue Sep 9, 2020
Implement dup and close for stdin/stdout/stderr

Implements some of the operations for stdin/out/err, towards #1499
bors added a commit that referenced this issue Sep 9, 2020
Implement dup and close for stdin/stdout/stderr

Implements some of the operations for stdin/out/err, towards #1499
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-shims Area: This affects the external function shims C-enhancement Category: a PR with an enhancement or an issue tracking an accepted enhancement
Projects
None yet
Development

No branches or pull requests

3 participants