dd: interpret Stdin as File to seek it#3662
dd: interpret Stdin as File to seek it#3662ArtemSkrebkov wants to merge 4 commits intouutils:mainfrom
Conversation
|
Could you please add a test? |
| #[cfg(any(target_family = "unix", target_family = "wasi"))] | ||
| let mut seekable_src = unsafe { | ||
| use std::os::unix::io::{AsRawFd, FromRawFd}; | ||
| std::fs::File::from_raw_fd(std::io::stdin().as_raw_fd()) |
There was a problem hiding this comment.
(driveby comment, I don't have much context) Unless seekable_src is fed to mem::forget at some point this likely is a bad idea because File::drop will then close the stdin file descriptor. You should either try_clone (dup) it or wrap this in a ManuallyDrop.
There was a problem hiding this comment.
That makes sense. Will fix it once I figure out why the current solution works if I use dd in command line and does not work if I run tests from tests/by-utils.
ArtemSkrebkov
force-pushed
the
stop_dd_consume_whole_stdin
branch
2 times, most recently
from
702588c to
d5be396
Compare
| } | ||
|
|
||
| #[test] | ||
| fn test_partial_skip_stdin() { |
There was a problem hiding this comment.
Well... I added the test, but it does not work as expected and fails with Illegal seek. As any other test which tries to seek stdin.
However, it can work outside the test system (for example, if I run tests/misc/head-c.sh)
I am confused why it does not work in the test system. Let me know if there are any insights on this.
| #[cfg(any(target_family = "unix", target_family = "wasi"))] | ||
| let mut seekable_src = unsafe { | ||
| use std::os::unix::io::{AsRawFd, FromRawFd}; | ||
| std::fs::File::from_raw_fd(std::io::stdin().as_raw_fd()) |
There was a problem hiding this comment.
That makes sense. Will fix it once I figure out why the current solution works if I use dd in command line and does not work if I run tests from tests/by-utils.
|
Could you please fix that issue? thanks |
Fixed. |
|
a bunch of tests are broken, could you please have a look? thanks |
|
Yeah. Right. I described why the tests fail in the previous comment. I will try to debug it more, but if there is someone, who is more proficient in the project, I can chat with, that would probably help to fix the issue faster. |
sylvestre
force-pushed
the
stop_dd_consume_whole_stdin
branch
2 times, most recently
from
4b406a2 to
34956b5
Compare
sylvestre
force-pushed
the
stop_dd_consume_whole_stdin
branch
2 times, most recently
from
b30d43b to
12a7123
Compare
sylvestre
force-pushed
the
stop_dd_consume_whole_stdin
branch
from
12a7123 to
44cbf17
Compare
sylvestre
left a comment
sylvestre
left a comment
There was a problem hiding this comment.
fails to build on a bunch of jobs
|
@ArtemSkrebkov ping? |
|
No luck since the last debugging attempt. |
|
Closing until better times :) |
4 participants
It fixes a test including
ddfromtests/misc/head-c.sh.Corresponding issue: #3008
I expected that
tests/dd/not-rewound.shwould also be fixed, but unfortunately, it is not the case. It seems that #3626 is still relevant.Background
An attempt to skip bytes by
read_skipleads to consuming all stdin by a rust application. Another approach to tackle this is to interpretstdinasFile. More details in #3008 (comment)