Skip to content

dd: exit when bs/ibs/obs/cbs isn't positive#11630

Open
iburaky2 wants to merge 2 commits intouutils:mainfrom
iburaky2:fix/dd-bs-not-positive
Open

dd: exit when bs/ibs/obs/cbs isn't positive#11630
iburaky2 wants to merge 2 commits intouutils:mainfrom
iburaky2:fix/dd-bs-not-positive

Conversation

@iburaky2
Copy link
Copy Markdown
Contributor

@iburaky2 iburaky2 commented Apr 4, 2026

Match GNU coreutils behavior:

$ dd bs=0
dd: invalid number: '0'
$ dd ibs=0
dd: invalid number: '0'
$ dd obs=0
dd: invalid number: '0'
$ dd cbs=0
dd: invalid number: '0'

$ dd bs=-5
dd: invalid number: '-5'
$ dd ibs=-5
dd: invalid number: '-5'
$ dd obs=-5
dd: invalid number: '-5'
$ dd cbs=-5
dd: invalid number: '-5'

Currently:

Another option would be to check in parse_bytes / parse_bytes_with_opt_multiplier but I think it's better to exit earlier.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 4, 2026

GNU testsuite comparison:

Note: The gnu test tests/csplit/csplit-heap is now being skipped but was previously passing.
Note: The gnu test tests/dd/no-allocate is now being skipped but was previously passing.
Note: The gnu test tests/tail/tail-n0f is now being skipped but was previously passing.

@oech3
Copy link
Copy Markdown
Contributor

oech3 commented Apr 4, 2026

Our dd already rejects negative value. So this PR might have duplicated logic with it.

@oech3
Copy link
Copy Markdown
Contributor

oech3 commented Apr 4, 2026

Also clippy is failing.

@iburaky2 iburaky2 force-pushed the fix/dd-bs-not-positive branch from 6f89772 to 3fae276 Compare April 4, 2026 01:54
@iburaky2
Copy link
Copy Markdown
Contributor Author

iburaky2 commented Apr 4, 2026

mb I got the order wrong parse_bytes comes before validate(). Since parse_bytes is only used with those 4 parameters and all should reject 0 we can handle the check there.

Negative numbers are handled here:

fn parse_bytes_no_x(full: &str, s: &str) -> Result<u64, ParseError> {
    let parser = SizeParser {
        capital_b_bytes: true,
        no_empty_numeric: true,
        ..Default::default()
    };
    let (num, multiplier) = match (s.find('c'), s.rfind('w'), s.rfind('b')) {
        (None, None, None) => match parser.parse_u64(s) { // -5 can't be parsed as y64
            Ok(n) => (n, 1),
            Err(ParseSizeError::SizeTooBig(_)) => (u64::MAX, 1),
            Err(_) => return Err(ParseError::InvalidNumber(full.to_string())), // error here
        },
        (Some(i), None, None) => (parse_bytes_only(s, i)?, 1),
        (None, Some(i), None) => (parse_bytes_only(s, i)?, 2),
        (None, None, Some(i)) => (parse_bytes_only(s, i)?, 512),
        _ => return Err(ParseError::MultiplierStringParseFailure(full.to_string())),
    };
    num.checked_mul(multiplier)
        .ok_or_else(|| ParseError::MultiplierStringOverflow(full.to_string()))
}

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 4, 2026

GNU testsuite comparison:

Skip an intermittent issue tests/pr/bounded-memory (was skipped on 'main', now failing)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants