dd: should terminate with error if skip argument is too large

[ic3man5]

Fixes #7216

 cargo run dd bs=1 skip=9223372036854775808 count=0
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.11s
     Running `target/debug/coreutils dd bs=1 skip=9223372036854775808 count=0`
dd: invalid number: ‘'9223372036854775808': Value too large for defined data type’

[ic3man5]

Notes:

• slight output difference: dd: invalid number: ‘'9223372036854775808': Value too large for defined data type’ instead of: dd: invalid number: '9223372036854775808': Value too large for defined data type
• seek also had this issue. For some reason GNU coreutils uses a signed integer and checks to make sure its in range of 0 and max i64.

[ic3man5]

reference to the check in GNU coreutils: https://github.com/coreutils/coreutils/blob/master/src/dd.c#L1581C23-L1581C39

[github-actions[bot]]

GNU testsuite comparison:

Skip an intermittent issue tests/misc/stdbuf (fails in this run but passes in the 'main' branch)

[cakebaker]

Can you please add a test to ensure we don't regress in the future?

[ic3man5]

Can you please add a test to ensure we don't regress in the future?

yes, do we care that the GnuTests CI step failed? I'm not sure how to tell what test exactly failed from the logs.

I ran this gnu test but I'm unsure of how to compare it to see what test actually failed.

$ bash util/run-gnu-test.sh

============================================================================
Testsuite summary for GNU coreutils 9.6.8-fbfd88-dirty
============================================================================
# TOTAL: 617
# PASS:  469
# SKIP:  85
# XFAIL: 0
# FAIL:  63
# XPASS: 0
# ERROR: 0
============================================================================
See ./tests/test-suite.log for debugging.
Some test(s) failed.  Please report this to [email protected],
together with the test-suite.log file (gzipped) and your system
information.  Thanks.
============================================================================

[cakebaker]

do we care that the GnuTests CI step failed?

It fails because of an intermittent test and you can ignore it.

[github-actions[bot]]

GNU testsuite comparison:

Skip an intermittent issue tests/misc/stdbuf (fails in this run but passes in the 'main' branch)

[ic3man5]

@cakebaker this should be ready to go now.

[github-actions[bot]]

GNU testsuite comparison:

Skip an intermittent issue tests/tail/inotify-dir-recreate (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/rm/rm2 is no longer failing!

[github-actions[bot]]

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

[RenjiSann]

The CI is unhappy :(

[github-actions[bot]]

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

[RenjiSann]

Can you handle this ?

  ──── TRY 1 STDERR:       coreutils::tests test_dd::test_invalid_number_arg_gnu_compatibility
  
  thread 'test_dd::test_invalid_number_arg_gnu_compatibility' panicked at tests/by-util/test_dd.rs:1274:14:
  assertion failed: `(left == right)`
  
  Diff < left / right > :
  <dd: invalid number: ‘''’
  >dd: invalid number: ‘’

[ic3man5]

sorry I didn't see notification on this one, I'll look into it.

[ic3man5]

@RenjiSann Okay I actually need some direction here. I noticed a lot of tests are quoting with ‘ (0x2018 Left Single Quotation Mark) and ’ (0x2019 Right Single Quotation Mark). Running coreutils on my system (arch linux) dd returns single quote character ' (0x27 Single Quotation).

I see a lot of compatibility issues here in other places besides this. What am I missing here?

coreutils installed on my system:

$ dd --version
dd (coreutils) 9.6

edit: The original issue report shows single quotations also as expected.

edit2: locale information

$ locale
LANG=en_US.UTF-8
LC_CTYPE="en_US.UTF-8"
LC_NUMERIC=en_US.UTF-8
LC_TIME=en_US.UTF-8
LC_COLLATE="en_US.UTF-8"
LC_MONETARY=en_US.UTF-8
LC_MESSAGES="en_US.UTF-8"
LC_PAPER=en_US.UTF-8
LC_NAME=en_US.UTF-8
LC_ADDRESS=en_US.UTF-8
LC_TELEPHONE=en_US.UTF-8
LC_MEASUREMENT=en_US.UTF-8
LC_IDENTIFICATION=en_US.UTF-8
LC_ALL=

[github-actions[bot]]

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/stdbuf (passes in this run but fails in the 'main' branch)

[RenjiSann]

@RenjiSann Okay I actually need some direction here. I noticed a lot of tests are quoting with ‘ (0x2018 Left Single Quotation Mark) and ’ (0x2019 Right Single Quotation Mark). Running coreutils on my system (arch linux) dd returns single quote character ' (0x27 Single Quotation).

I see a lot of compatibility issues here in other places besides this. What am I missing here?

coreutils installed on my system:

$ dd --version
dd (coreutils) 9.6

edit: The original issue report shows single quotations also as expected.

edit2: locale information

$ locale
LANG=en_US.UTF-8
LC_CTYPE="en_US.UTF-8"
LC_NUMERIC=en_US.UTF-8
LC_TIME=en_US.UTF-8
LC_COLLATE="en_US.UTF-8"
LC_MONETARY=en_US.UTF-8
LC_MESSAGES="en_US.UTF-8"
LC_PAPER=en_US.UTF-8
LC_NAME=en_US.UTF-8
LC_ADDRESS=en_US.UTF-8
LC_TELEPHONE=en_US.UTF-8
LC_MEASUREMENT=en_US.UTF-8
LC_IDENTIFICATION=en_US.UTF-8
LC_ALL=

The reason is that for now, we don't handle locales at all, and by default we replicate the behavior of LC_ALL=C.

[RenjiSann]

Consequently, the easiest thing to do right now is to expect our code to format output as expected with the LC_ALL=C locale.

Someday we'll look into localization, but there is no easy way to handle it for now.

[RenjiSann]

ping @ic3man5 there are some conflicts

[ic3man5]

sorry, going to keep working on this, just been busy. If someone else wants to take it over I won't mind. I don't want to block progress.

[ic3man5]

@RenjiSann Updated and fixed. Should be good to go now.

$ cargo run dd bs=1 skip=9223372036854775808 count=0
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.11s
     Running `target/debug/coreutils dd bs=1 skip=9223372036854775808 count=0`
dd: invalid number: ‘9223372036854775808’: Value too large for defined data type

$ cargo test --package coreutils --test tests -- test_dd::test_skip_overflow
    Finished `test` profile [unoptimized + debuginfo] target(s) in 0.12s
     Running tests/tests.rs (target/debug/deps/tests-025d24597495cb89)

running 1 test
test test_dd::test_skip_overflow ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 3495 filtered out; finished in 0.01s

[github-actions[bot]]

GNU testsuite comparison:

Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

[ic3man5]

@cakebaker @sylvestre I took so long to finish this merge request the code reviewers here might not be active right now. Tagging you two just so this doesn't get lost. Sorry for the noise if unnecessary.

[mfontanaar]

I've just did a (conflictless) rebase and added the exact same test as a separate commit here: https://github.com/mfontanaar/coreutils-7216/commits/issue_7216/

I was skeptical of doing it, before reading this:

sorry, going to keep working on this, just been busy. If someone else wants to take it over I won't mind. I don't want to block progress.

Unfortunately, I think the source branch for a PR cannot be changed, and opening a separate PR for this minuscule addition of my own would be very unfair for @ic3man5, who did all the work. Hence I'm leaving this message for completion and having something mergeable, if it comes to it.

[RenjiSann]

@mfontanaar Maybe you can create a new merge request with both commits (yours and @ic3man5's) so we can finally merge this?

[mfontanaar]

@mfontanaar Maybe you can create a new merge request with both commits (yours and @ic3man5's) so we can finally merge this?

@RenjiSann: Done