-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
dd: buffer partial blocks in the output writer #4545
Conversation
GNU testsuite comparison:
|
tests/by-util/test_dd.rs
Outdated
|
||
UCommand::new() | ||
.arg(format!( | ||
"({TESTS_BINARY} printf 'ab'; {TESTS_BINARY} sleep 0.1; {TESTS_BINARY} printf 'cd') > {fifoname}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These tests are failing for reasons I don't understand yet. It seems that the two bytes "ab" and the two bytes "cd" are sometimes being read by dd
one byte at a time, resulting in one partial block read comprising the byte "a" and a separate partial block read comprising the byte "b" (or "c" and "d", respectively). It seems to be happening only when I run a large set of tests, like cargo test test_dd
, and not when I run the tests individually or as a pair, as in cargo test test_dd::test_reading_partial_blocks_from_fifo
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I switched to using explicit Command
, let's see if that helps.
GNU testsuite comparison:
|
@jfinkels ping here too ? :) |
Thanks, I'll plan to look again at this soon
…------- Original Message -------
On Friday, April 28th, 2023 at 3:35 AM, Sylvestre Ledru ***@***.***> wrote:
***@***.***(https://github.com/jfinkels) ping here too ? :)
—
Reply to this email directly, [view it on GitHub](#4545 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AAA5XG6GB6VEIJPZTBUK6GTXDNXL3ANCNFSM6AAAAAAV7UUX7M).
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
GNU testsuite comparison:
|
1 similar comment
GNU testsuite comparison:
|
@jfinkels it is conflicting with your other pr, sorry |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need rebasing
I'm starting to have some more time again, so I'll take another look at this. Thanks for your patience |
GNU testsuite comparison:
|
GNU testsuite comparison:
|
GNU testsuite comparison:
|
@jfinkels sorry it is taking too long :/ |
GNU testsuite comparison:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for getting back to this!
GNU testsuite comparison:
|
7f5fd67
to
0e3f1e3
Compare
GNU testsuite comparison:
|
GNU testsuite comparison:
|
GNU testsuite comparison:
|
Looks like the remaining build failures are known issues:
So this should be reviewed again. |
GNU testsuite comparison:
|
GNU testsuite comparison:
|
Add the `Settings.buffered` field to indicate whether partial output blocks should be buffered until they are complete.
Correct the behavior of `dd` so that the termination condition of the main loop uses the number of bytes read, not the number of bytes written, when the `count` command-line option is given in bytes instead of blocks.
Add buffering of partial blocks in the output block writer until they are completed.
GNU testsuite comparison:
|
Impressive number of tests, bravo :) |
we even this that at the coverage level |
This pull request adds buffering of partial blocks in the output block writer until they are completed.
The test case that demonstrates this buffering behavior is
The "ab" forms a partial block, which is buffered and not written immediately. Then when the "cd" comes in, the complete block "abc" is written, followed by the partial block "d".
To override the behavior set
bs=N
, like this:Fixes GNU test case
tests/dd/reblock.sh
.