fix(tac):gnu tac-2-nonseekable.sh test #9137
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add a new function `ensure_stdin_open()` that uses the `nix` crate to check if stdin is open before attempting to read from it. This prevents potential errors when stdin is closed, improving robustness of the tac command on Unix systems. Updated dependencies to include `nix` in Cargo.toml and Cargo.lock.
CodSpeed Performance ReportMerging #9137 will not alter performanceComparing Summary
Footnotes
|
|
GNU testsuite comparison: |
Contributor
|
the test doesn't seem to pass ?! |
Contributor
|
- Call path.metadata() once instead of multiple times to improve performance - Check if path is a directory or file using the single metadata result - Add test for FIFO (named pipe) input to ensure proper handling of non-regular files - This change reduces redundant system calls and enables tac to work with FIFOs by falling back to read() when not a regular file
- Remove unnecessary line breaks in OpenOptions and child.wait() calls for conciseness and readability.
Replace match statement with if-let-else for path.metadata() to leverage modern Rust syntax, improving code conciseness without altering functionality.
…rectly Zero-length files in procfs/sysfs may still produce data, but memory mapping them yields an empty buffer, causing incorrect behavior. This change adds a check for file length > 0 before attempting mmap, ensuring compatibility with GNU tac-2-nonseekable test.
Add "sysfs" to the cspell jargon dictionary to prevent spell check warnings for this Linux filesystem term used in the codebase.
|
GNU testsuite comparison: |
- Switch tac crate dependency from nix to libc - Implement direct libc-based checks for stdin state and /dev/null - Avoids nix dependency while preserving robust stdin error handling
- Replace CStr::from_bytes_with_nul with CString::new for /dev/null to simplify construction and clarify intent - Remove unused libc import, keeping unix-specific code minimal and cleaner
- Add ACCMODE, RDONLY, RDWR to cspell jargon dictionary to avoid false positives - Gate CString and MaybeUninit imports behind unix cfg to prevent non-unix build issues
|
GNU testsuite comparison: |
sylvestre
reviewed
Nov 8, 2025
sylvestre
reviewed
Nov 8, 2025
- Add nix as a unix-only dependency for tac - Replace direct libc fcntl/open/fstat calls with nix equivalents - Simplify stdin/dev-null checks for improved safety and clarity
- Remove libc dependency from tac crate and Cargo.lock - Use nix OFlag/FileStat and std::os::fd::AsRawFd for stdin checks - Improve error handling by mapping nix Errno to std::io::Error
- Reorders nix::fcntl imports to follow a consistent, conventional order - Improves code readability without changing behavior
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
Collaborator
|
Does this make it change compatibility for when the input is /dev/null ? |
- Simplify stdin closure check by removing /dev/null specific logic - Remove unused functions and imports - Add test ensuring RDWR /dev/null stdin is supported
|
GNU testsuite comparison: |
…tion Remove dependency on nix crate and use uucore's new signals feature to detect if stdin was closed before Rust reopens it as /dev/null. This centralizes stdio state handling and improves consistency across utilities. Add libc dependency for signal functions. Update error handling to set exit code properly on stdin closure.
|
GNU testsuite comparison: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add a new function
ensure_stdin_open()that uses thenixcrate to check if stdin is open before attempting to read from it. This prevents potential errors when stdin is closed, improving robustness of the tac command on Unix systems. Updated dependencies to includenixin Cargo.toml and Cargo.lock.relate
#9127