truncate: correctly handle file (non-)creation#6108
Conversation
src/uu/truncate/src/truncate.rs
Outdated
| return Err(USimpleError::new( | ||
| 1, | ||
| format!( | ||
| "cannot open {} for writing: No such device or address", |
There was a problem hiding this comment.
This error message is indeed a clear indication that we're doing something wrong. We should be making a syscall that returns "No such device or address". That shouldn't block merging this PR though, we can indeed fix that later if you don't want to do it now.
strace shows this call as the one generating that error:
openat(AT_FDCWD, "blabla", O_WRONLY|O_NONBLOCK) = -1 ENXIO (No such device or address)
This Rust code:
OpenOptions::new().write(true).create(true).open("myfifo");actually does the following syscall:
openat(AT_FDCWD, "myfifo", O_WRONLY|O_CLOEXEC)
Looks like the O_NONBLOCK is the culprit there. So maybe we can just fix this by setting the same mode as GNU via OpenOptionsExt and the appropriate libc constants?
This got me the right error message:
OpenOptions::new().write(true).custom_flags(libc::O_NONBLOCK).open("myfifo");There was a problem hiding this comment.
Yes, opening the file in non-blocking mode is probably the right way forward, and your code looks correct. However, I don't want to fix this particular aspect in this PR; please note that this fifo-toctou-bug already existed before this PR, and remains unchanged.
|
GNU testsuite comparison: |
|
Oops! Looks like this broke everything on windows. |
The fifo check used to include 'metadata(filename)?', which would error if the file does not exist. In our case however, this is not an error.
ad34bf9 to
a1ad751
Compare
|
Changes since last push:
|
|
GNU testsuite comparison: |
|
excellent! i guess it is ready ? |
This PR fixes three issues:
test_referenceandtest_truncate_bytes_sizehad dead code with comments "Uncomment when XYZ is implemented", but XYZ was already implemented two years ago. So I uncommented it.--no-create. So I moved the code from the "size-only" implementation to the common function.Note that
truncatestill has a TOCTOU bug: The fifo check is done on the path, not on the file(descriptor), which leaves a (very short) window of time where a regular file could be swapped out for a fifo. The "correct way" is probably to drop the fifo check entirely and instead open the file in a special non-blocking way: https://users.rust-lang.org/t/opening-a-fifo-for-writing-hangs/107738 However, I'm not gonna touch that.