dd: handle stdout redirected to seekable file #3880
Conversation
jfinkels
force-pushed
the
dd-redirected-stdout
branch
from
269e533 to
c737d2a
Compare
| /// Canonicalized file name of `/dev/stdout`. | ||
| /// | ||
| /// For example, if this process were invoked from the command line as | ||
| /// `dd`, then this function returns the [`OsString`] form of | ||
| /// `"/dev/stdout"`. However, if this process were invoked as `dd > | ||
| /// outfile`, then this function returns the canonicalized path to | ||
| /// `outfile`, something like `"/path/to/outfile"`. | ||
| fn stdout_canonicalized() -> OsString { | ||
| match Path::new("/dev/stdout").canonicalize() { | ||
| Ok(p) => p.into_os_string(), | ||
| Err(_) => OsString::from("/dev/stdout"), | ||
| } | ||
| } | ||
|
|
||
| /// Decide whether stdout is being redirected to a seekable file. | ||
| /// | ||
| /// For example, if this process were invoked from the command line as | ||
| /// | ||
| /// ```sh | ||
| /// dd if=/dev/zero bs=1 count=10 seek=5 > /dev/sda1 | ||
| /// ``` | ||
| /// | ||
| /// where `/dev/sda1` is a seekable block device then this function | ||
| /// would return true. If invoked as | ||
| /// | ||
| /// ```sh | ||
| /// dd if=/dev/zero bs=1 count=10 seek=5 | ||
| /// ``` | ||
| /// | ||
| /// then this function would return false. | ||
| fn is_stdout_redirected_to_seekable_file() -> bool { | ||
| let s = stdout_canonicalized(); | ||
| let p = Path::new(&s); | ||
| match File::open(p) { | ||
| Ok(mut f) => { | ||
| f.seek(SeekFrom::Current(0)).is_ok() | ||
| && f.seek(SeekFrom::End(0)).is_ok() | ||
| && f.seek(SeekFrom::Start(0)).is_ok() | ||
| } | ||
| Err(_) => false, | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
There's similar functionality in other uutils, e.g. tail. We should think about a follow-up PR to move this kind of checks to a central file to reduce code duplication.
|
I know you had serious problems with this on your system before, so sorry for asking. Did you try to run |
|
I did not try to run that test yet. I thought the CI system executes that test because of the issues we had in #3442, but now it looks like the test is skipped? From the logs: |
|
Yes, sadly the CI doesn't run any GNU tests which requires root. That's why I asked. |
|
Okay, I'll install a virtual machine and try it there. |
|
I checked out the branch from pull request #3882 (which includes the changes from this branch as well as the change to the relevant part of the error log is: The message "failed to seek in output file: Invalid input" comes from Lines 543 to 544 in 282774a Output<File>. That's what I expected, so that's good!
|
Move the argument parsing code out of the `Input::new()` and `Output::new()` functions and into the calling code. This allows the calling code to make decisions about how to instantiate the `Input` and `Output` objects if necessary.
Fix a bug in `dd` where null bytes would be unintentionally written if
stdout were redirected to a seekable file. For example, before this
commit, if `dd` were invoked from the command-line as
dd if=infile bs=1 count=10 seek=5 > /dev/sda1
then five zeros would be written to `/dev/sda1` before copying ten
bytes of `infile` to `/dev/sda1`. After this commit, `dd` will
correctly seek five bytes forward in `/dev/sda1` before copying the
ten bytes of `infile`.
Fixes uutils#3542.
sylvestre
force-pushed
the
dd-redirected-stdout
branch
from
c737d2a to
59e3d9c
Compare
This pull request fixes a bug in
ddwhere null bytes would be unintentionally written if stdout were redirected to a seekable file. For example, before this commit, ifddwere invoked from the command-line asthen five zeros would be written to
/dev/sda1before copying ten bytes ofinfileto/dev/sda1. After this commit,ddwill correctly seek five bytes forward in/dev/sda1before copying the ten bytes ofinfile. I wasn't sure how to write a test for this.In order to accomplish this, the first commit moves the argument parsing out of the
Input::new()andOutput::new()functions. This was needed in order to be able to determine whether we need to use theOutput<File>specialization even when no outfile is specified in the command-line arguments. The code is a bit messy, but I think it should work.Fixes #3542.