tail: do not abort on a -c +N offset past the end of the file - #13901
tail: do not abort on a -c +N offset past the end of the file#13901AlejandroCoronadoN wants to merge 1 commit into
Conversation
163202f to
2592029
Compare
Merging this PR will improve performance by 3.21%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | Simulation | ls_recursive_balanced_tree[(6, 4, 15)] |
120.7 ms | 117 ms | +3.21% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing AlejandroCoronadoN:fix-tail-c-plus-seek-panic (3f5f713) with main (d8bee62)
Footnotes
-
50 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. ↩
|
please fix the lint: |
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Prevents tail -c +N from panicking when the requested start offset is past EOF and the underlying seek fails (notably for offsets exceeding i64::MAX), matching GNU tail behavior.
Changes:
- Replace an
unwrap()on a forward seek with a graceful fallback to seeking to EOF when the seek fails. - Add a regression test covering very large positive byte offsets past end-of-file.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/uu/tail/src/tail.rs |
Avoids aborting on failed forward seek by falling back to EOF for -c +N. |
tests/by-util/test_tail.rs |
Adds a regression test ensuring large -c +N offsets do not crash and produce no output. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if file.seek(SeekFrom::Start(*count - 1)).is_err() { | ||
| file.seek(SeekFrom::End(0))?; | ||
| } |
| at.write("f", &"a".repeat(1_000_000)); | ||
| ucmd.args(&["-c", "+18446744073709551615", "f"]) |
2592029 to
9fa6b88
Compare
|
GNU testsuite comparison: |
9fa6b88 to
3f5f713
Compare
|
Thanks for the review. Fixed the lint by replacing the if/is_err with a .or_else fallback on the seek; clippy and fmt are clean now. |
|
Please make the comments shorter |
tail -c +N seeked to byte N-1 and unwrapped the result, so an offset past the end of the file (e.g. above i64::MAX) aborted. Fall back to seeking to the end so nothing is printed, matching GNU, instead of panicking.
|
Shortened the comment. Thanks! |
3f5f713 to
13e52c4
Compare
Closes #13887.
Problem
bounded_taildoesfile.seek(SeekFrom::Start(*count - 1)).unwrap()for apositive byte offset. When the offset is past the end of the file and not
seekable (here it exceeds
i64::MAX), the seek returns an error and the unwrapaborts. GNU
tailprints nothing for a start offset past end of file and exits0.
Fix
Mirror the existing negative-bytes arm: if the forward seek fails, fall back to
SeekFrom::End(0)so the following copy reads nothing, instead of unwrapping.Verification
Compared against GNU
tailover offsets+18446744073709551615,+17592186040322,+8193,+100: exit codes and byte counts match. Added aregression test; the full
test_tailsuite (122 tests) passes andcargo fmt/
cargo clippyare clean.