Skip to content

csplit print 0 on an error like gnu does#13206

Open
HackingRepo wants to merge 6 commits into
uutils:mainfrom
HackingRepo:patch-2
Open

csplit print 0 on an error like gnu does#13206
HackingRepo wants to merge 6 commits into
uutils:mainfrom
HackingRepo:patch-2

Conversation

@HackingRepo

Copy link
Copy Markdown

fixes #13139

@codspeed-hq

codspeed-hq Bot commented Jun 29, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 4.67%

❌ 1 regressed benchmark
✅ 326 untouched benchmarks
⏩ 46 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation sort_ascii_c_locale 16 ms 16.7 ms -4.67%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing HackingRepo:patch-2 (ec8fb45) with main (b76d615)

Open in CodSpeed

Footnotes

  1. 46 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Comment thread src/uu/csplit/src/csplit.rs Outdated
if let Err(ref e) = res {
uucore::show_error!("{}", e);
println!("0");
std::process::exit(1);

This comment was marked as resolved.

Comment thread src/uu/csplit/src/csplit.rs Outdated
Comment on lines +642 to +649
let file = File::open(file_name)
.map_err_context(|| format!("cannot open {} for reading", file_name.quote()))?;
Ok(csplit(&options, &patterns, BufReader::new(file))?)
.map_err_context(|| format!("cannot open {} for reading", file_name.quote()));

match file {
Ok(f) => csplit(&options, &patterns, BufReader::new(f)).map_err(Into::into),
Err(e) => Err(e),
}
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably more something like:

  let res = if file_name == "-" {
      let stdin = io::stdin();
      csplit(&options, &patterns, stdin.lock()).map_err(Into::into)
  } else {
      File::open(file_name)
          .map_err_context(|| format!("cannot open {} for reading", file_name.quote()))
          .and_then(|file| csplit(&options, &patterns, BufReader::new(file)).map_err(Into::into))
  };

@HackingRepo HackingRepo requested a review from oech3 June 29, 2026 14:16
@github-actions

github-actions Bot commented Jun 29, 2026

Copy link
Copy Markdown

GNU testsuite comparison:

Skip an intermittent issue tests/cut/bounded-memory (fails in this run but passes in the 'main' branch)
Note: The gnu test tests/csplit/csplit-heap is now being skipped but was previously passing.
Congrats! The gnu test tests/printf/printf-surprise is now passing!
@HackingRepo HackingRepo requested a review from sylvestre June 29, 2026 14:20
// Print 0 second
let stdout = io::stdout();
let mut handle = stdout.lock();
let _ = writeln!(handle, "0");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GNU catches write error of stdout and ignore write error of stderr (with few expections).
Also lock() is not useful since write is done just once at here.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so if that, that was a bit complicated, not all errors gnu print 0, that was the issue making it harder, what type of error it do that, we need known

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

Labels

None yet

3 participants