Skip to content

tail: report a read/seek failure instead of panicking - #14298

Open
arbelonson-source wants to merge 1 commit into
uutils:mainfrom
arbelonson-source:fix/tail-io-error-panic
Open

tail: report a read/seek failure instead of panicking#14298
arbelonson-source wants to merge 1 commit into
uutils:mainfrom
arbelonson-source:fix/tail-io-error-panic

Conversation

@arbelonson-source

Copy link
Copy Markdown
Contributor

Fixes #13124.

Once bounded_tail decides a file is seekable, every seek and read it does to find the start of the last N lines/bytes assumed success and unwrapped the result. A seekable file that then fails to read — a character device that times out or errors mid-read, for instance — crashed instead of reporting the failure:

$ sudo tail /dev/drm_dp_aux2          # ours, before
thread 'main' panicked at src/uu/tail/src/chunks.rs:92:40:
called `Result::unwrap()` on an `Err` value: Os { code: 5, kind: Uncategorized, message: "Input/output error" }

$ sudo tail /dev/drm_dp_aux2          # GNU 9.11
tail: error reading '/dev/drm_dp_aux2': Input/output error

This isn't specific to -n (the mode in the original report) or to this one device — it's every mode that takes the seekable/bounded path, since they all shared the same unwrap-on-seek pattern:

$ sudo tail -n +5 /dev/drm_dp_aux2    # panicked at tail.rs, now: matches GNU
$ sudo tail -c 5  /dev/drm_dp_aux2    # did not panic (different, already-fallible path), but said only "Input/output error" with no filename

What changed

  • ReverseChunks::new and its Iterator::next (used by -n's negative case) now return io::Result instead of unwrapping every seek/read.
  • backwards_thru_file propagates that with ? instead of .unwrap().
  • bounded_tail's seeking logic (covering all of -n/-c, positive and negative) is split into a small io::Result-returning helper, so every one of its seeks propagates instead of unwrapping.
  • The one call site converts an io::Error from that helper into the same tail-error-reading-file message tail already uses for an unreadable directory, so the wording and exit code (1) match GNU exactly.

Not covered

The -c cases didn't panic before this change — print_target_section's copy path is already fallible — but they report a bare Input/output error without the filename, where GNU says error reading 'FILE': .... That gap is unchanged by this PR; fixing it means threading the display name into print_target_section too, which felt like a separate, smaller follow-up rather than something to fold in here.

Testing

Verified against /dev/drm_dp_aux2 as root (the device from the report), across every bounded_tail mode: default, -n N, -n +N, -c N, -c +N. Each now matches GNU's message and exit code exactly, where all but the -c cases previously panicked.

I couldn't find a portable, root-free way to reproduce the underlying condition (a seekable file whose read genuinely fails) in the shared test suite, so this is verified manually rather than by an added regression test.

  • 16-case differential sweep against GNU 9.11 on ordinary files (all -n/-c sign/zero combinations, small and large files, nonexistent files): all match, confirming no regression to the normal path.
  • cargo test --features tail --test tests -- test_tail: 161 passed, 0 failed (all pre-existing; unaffected by this change).
  • Also compiles and passes under the full multicall cargo test --test tests build.
  • cargo fmt --check and cargo clippy -p uu_tail --all-targets -- -D warnings: clean.

Disclosure

Prepared with AI assistance (Claude Opus 5, via Claude Code), per the AI policy in CONTRIBUTING.md. GNU's behaviour was established by running the installed GNU binary as a black box; I did not read GNU coreutils source. All testing was run locally.

Once `bounded_tail` decides a file is seekable, every seek and read it
does to find the start of the last N lines/bytes assumed success and
unwrapped the result. A seekable file that then fails to read -- a
character device that times out or errors mid-read, for instance --
crashed instead of reporting the failure:

    $ tail /dev/drm_dp_aux2
    thread 'main' panicked at src/uu/tail/src/chunks.rs:92:40:
    called `Result::unwrap()` on an `Err` value: Os { code: 5, ... }

GNU reports it as a read error and exits 1:

    $ tail /dev/drm_dp_aux2
    tail: error reading '/dev/drm_dp_aux2': Input/output error

`ReverseChunks` (used by `-n`'s negative case) and every seek in
`bounded_tail` (used by all of `-n`/`-c`, positive and negative) now
return `io::Result` instead of unwrapping, converted at the one call
site into the same message `tail` already uses for an unreadable
directory.

Verified against the same device as root, across every `bounded_tail`
mode (default, `-n N`, `-n +N`, `-c N`, `-c +N`): each now matches GNU's
message and exit code exactly, where each previously panicked (the `-c`
cases already avoided a panic before this change, by going through a
different, already-fallible copy path, but reported a bare
"Input/output error" without the filename; that gap is unchanged here
and is a separate, pre-existing issue).

No portable, root-free way to reproduce the underlying condition in the
test suite -- it needs a real device whose read genuinely fails after
open -- so this is verified manually rather than by an added test;
cargo test --features tail --test tests -- test_tail (161 pre-existing,
0 new) is unaffected.
@github-actions

Copy link
Copy Markdown

GNU testsuite comparison:

Skip an intermittent issue tests/tail/symlink (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/cut/bounded-memory (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/date/date-locale-hour (passes in this run but fails in the 'main' branch)
Note: The gnu test tests/cut/cut-huge-range is now being skipped but was previously passing.
Note: The gnu test tests/rm/many-dir-entries-vs-OOM is now being skipped but was previously passing.
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.

bug(tail): when the filename is /dev/drm_dp_aux2 panics

1 participant