cat: diagnose splice errors - #13812
Conversation
Merging this PR will not alter performance
Comparing Footnotes
|
sylvestre
left a comment
There was a problem hiding this comment.
Please add a test to make sure we don't regress in the future
|
Done @sylvestre |
|
I don't agree with this approach. This should be shared at uucore for all utils. Also distioction for read/write is not splice specific. |
|
Why cannot we simply use |
Note that error distinction is disabled until next releas of GNU at backporting at https://github.com/uutils/coreutils/blob/main/util/fetch-gnu.sh . |
Hmm, but not for me? For me locally without pr it doesn't pass. |
|
GNU testsuite comparison: |
|
I still don't understand this large diff just for appending a new message to existing error message. |
Function, it is (will move to uucore once your prs are done). Enum, no, because it is the idiomatic way as Errors should be in a large enum. Macro, well sylvestre asked for it. |
|
We should remove |
Isn't that a good thing? In uucore it can have its own error, though CatResult and CatError isn't created by me, so its the maintainers decision. |
|
If we want to share logic for error with file path and distinction, utility specific abstriction is large barrier for sharing code base in my opinion. |
xtqqczze
left a comment
There was a problem hiding this comment.
I don’t have any further comments regarding this PR.
6d8699c to
5c18853
Compare
|
I still don't like this PR with too large diff and unshared code base... |
|
I don't understand why is |
I just ran the cat tests at gnu master, and to be honest, splice.sh still fails: But the new cat-distinct-err.sh passes: IDK but only with my patches, even gnu master passes. @oech3 I will investigate in main branch myself to see whats happening.... |
The splice() fast path swallowed every error as a silent read/write fallback, so a splice failure after some bytes were copied (e.g. an EIO injected by strace in tests/cat/splice.sh) produced no diagnostic. Once any data has been copied, treat a splice error as fatal and report it as "cat: <input>: …" on the read side or "cat: write error: …" on the write side; failures before the first byte still fall back to read/write. Write errors elsewhere now also omit the input file name. Fixes tests/cat/splice.sh (backported in util/fetch-gnu.sh). Signed-off-by: MuntasirSZN <muntasir.joypurhat@gmail.com>
Signed-off-by: MuntasirSZN <muntasir.joypurhat@gmail.com>
Signed-off-by: MuntasirSZN <muntasir.joypurhat@gmail.com>
Signed-off-by: MuntasirSZN <muntasir.joypurhat@gmail.com>
Signed-off-by: MuntasirSZN <muntasir.joypurhat@gmail.com>
Signed-off-by: MuntasirSZN <muntasir.joypurhat@gmail.com>
5c18853 to
c2a4f9e
Compare
| #[inline] | ||
| fn splice_unusable_errno() -> std::io::Error { | ||
| std::io::Error::from_raw_os_error(rustix::io::Errno::INVAL.raw_os_error()) | ||
| } |
There was a problem hiding this comment.
I think both of them is unnecessary since we can modify/check errno.
| return Ok(Err(())); | ||
| match splice(pipe, dest, remaining) { | ||
| Ok(0) => { | ||
| // no progress; drain by hand |
There was a problem hiding this comment.
I believe this match arm is misundertnding.
|
@oech3 maybe I found the issue of why splice.sh is failing (as in fetch-gnu of uutils master): So GNU's rule: errors on the first splice cycle → silently fall back to read/write; errors on any later cycle → fatal with diagnosis (input → path, output → write error). uutils currently falls back on every splice error, thats the main issue. Pls correct me if I am wrong... uutils prints cat: /dev/zero: No space left on device for a write error; GNU prints cat: write error: No space left on device. Both stem from the missing Write error variant. |
|
I don't think so since the backported splice.sh's exit code does not depend on splice call completely.
|
Note
Needs #13847
The splice() fast path swallowed every error as a silent read/write
fallback, so a splice failure after some bytes were copied (e.g. an
EIO injected by strace in tests/cat/splice.sh) produced no diagnostic.
Once any data has been copied, treat a splice error as fatal and report
it as
cat: <input>: …on the read side orcat: write error: …on thewrite side; failures before the first byte still
fall back to read/write. Write errors elsewhere now also omit the input
file name.
Now tests/cat/splice.sh passes.
Signed-off-by: MuntasirSZN muntasir.joypurhat@gmail.com