split: do not overflow on -n l/K when K is larger than the byte count - #13903
split: do not overflow on -n l/K when K is larger than the byte count#13903AlejandroCoronadoN wants to merge 1 commit into
Conversation
Splitting into more line-based chunks than there are bytes (for example `split -n l/5` on a two-byte file) panicked with an integer overflow. When `num_chunks > num_bytes`, `chunk_size_base` is 0 and the trailing empty chunks add nothing to `num_bytes_should_be_written`, so the inner loop never advances past `num_bytes_written` and spins until `chunk_number` and `skipped` overflow. Stop the loop once the last chunk is reached, so the extra chunks are written as empty files, matching GNU split.
Merging this PR will degrade performance by 7.83%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | df_with_path |
573.7 µs | 704.9 µs | -18.62% |
| ⚡ | Simulation | du_summarize_balanced_tree[(5, 4, 10)] |
16.8 ms | 16.1 ms | +4.38% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing AlejandroCoronadoN:fix-split-n-lk-overflow (eed3fdb) with main (822aa83)
Footnotes
-
318 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. ↩
| num_bytes_written += num_line_bytes; | ||
| let mut skipped = -1; | ||
| while num_bytes_should_be_written <= num_bytes_written { | ||
| // Stop once the last chunk is reached. When there are more chunks than |
There was a problem hiding this comment.
I don't think we need 5 five comment lines
Problem
When there are more chunks than bytes,
chunk_size_base = num_bytes / num_chunksis 0 and the trailing empty chunks add 0 to
num_bytes_should_be_written, so theinner
while num_bytes_should_be_written <= num_bytes_writtenloop never advancesand spins until
chunk_number/skippedoverflow.Fix
Add
chunk_number < num_chunksto the loop condition so it stops at the lastchunk. There are only
num_chunkschunks, so this never truncates real output.Verification
Compared against GNU
splitfor-n l/5onab,-n l/3ona,-n l/2onempty input,
-n l/3on a 10-byte line, and multi-line inputs: the file set andcontents match exactly (
xaagets the data, the rest are empty). Added aregression test; the full
test_splitsuite (129 tests) passes andcargo fmt/
cargo clippyare clean.