Repository navigation
Conversation
isWritable tested the O_WRONLY bit, but O_RDWR is a separate access mode, so a file opened for reading and writing rejected every Write and CopyFrom. CreateTemp and OpenFile with O_RDWR on a PosixFS were affected; WinFS already handled O_RDWR. Compare the access mode instead of a single bit, and express isReadable the same way. Signed-off-by: Youssef Omar Bouden <youssef.bouden2002@gmail.com>
There was a problem hiding this comment.
🟢 Approval recommended
The implementation directly fixes the reported behavior and includes focused regression coverage.
0 open findings
What changed in this PR
Fixes POSIX file access-mode detection so O_RDWR handles support both reading and writing.
Changes:
- Adds a shared access-mode helper.
- Corrects readable/writable checks.
- Adds coverage for access modes and closed files.
| File | Description |
|---|---|
remotefs/posixfile.go |
Corrects POSIX access-mode handling. |
remotefs/posixfile_internal_test.go |
Tests access modes and closed handles. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
PosixFile.isWritable tested the O_WRONLY bit, but O_RDWR is a separate access mode (2), not a value that includes O_WRONLY (1). A file opened for reading and writing on a PosixFS was readable and not writable, so every Write and CopyFrom on it failed with "file ... is not open for writing". remotefs.CreateTemp, which opens with O_RDWR|O_CREATE|O_EXCL and documents the file as open for reading and writing, returned a handle that could not be written on POSIX hosts, and OpenFile with O_RDWR had the same problem. WinFS handles O_RDWR correctly, and Upload and WriteFile open with O_WRONLY, so they were not affected.
isWritable now compares the access mode, flags & (O_RDONLY|O_WRONLY|O_RDWR), and isReadable is written the same way. A table test covers the access modes, including O_RDWR, and a closed file.
The O_APPEND position handling noted in the issue is left for a separate change.
Fixes #481