-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replace OFlag & Mode with nix-independent type #389
Conversation
WalkthroughThe recent updates in the yash project enhance file handling by replacing outdated flag-based approaches with structured, type-safe enumerations for file access permissions. This transition improves code clarity and maintainability while introducing robust error handling. The reorganization of file system operations results in a more efficient and user-friendly shell environment, fostering better interactions with file descriptors. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Shell
participant System
User->>Shell: Request file operation
activate Shell
Shell->>System: Open file with OfdAccess
activate System
System-->>Shell: Return file descriptor
deactivate System
Shell-->>User: Confirm operation success
deactivate Shell
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
This commit migrates the System::open method to the new System::open2 method. In upcoming changes, the System::open method will be removed and the System::open2 method will be renamed to System::open.
Before renaming the new open2 method to open, we need to remove the existing open method. This commit removes the deprecated open method.
This method returns the open file description access mode. This method will replace the fcntl_getfl method.
This method is removed because it depends on the OFlag type, which is re-exported from the nix crate. To make our public API independent of the nix crate, we remove this method.
Before renaming the new Mode type, we need to remove the existing one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Outside diff range, codebase verification and nitpick comments (1)
yash-env/src/system/shared.rs (1)
Discrepancy in
get_and_set_nonblocking
implementationThe
get_and_set_nonblocking
method insystem/real.rs
correctly sets and restores the non-blocking state usingfcntl
system calls. However, the implementation insystem/virtual.rs
is currently a placeholder that always returnsfalse
and does not actually set or restore the non-blocking state.
system/real.rs
implementation (correct):fn get_and_set_nonblocking(&mut self, fd: Fd, nonblocking: bool) -> Result<bool> { let old_flags = unsafe { nix::libc::fcntl(fd.0, nix::libc::F_GETFL) }.errno_if_m1()?; let new_flags = if nonblocking { old_flags | nix::libc::O_NONBLOCK } else { old_flags & !nix::libc::O_NONBLOCK }; if new_flags != old_flags { unsafe { nix::libc::fcntl(fd.0, nix::libc::F_SETFL, new_flags) }.errno_if_m1()?; } let was_nonblocking = old_flags & nix::libc::O_NONBLOCK != 0; Ok(was_nonblocking) }
system/virtual.rs
implementation (placeholder):fn get_and_set_nonblocking(&mut self, fd: Fd, _nonblocking: bool) -> Result<bool> { self.with_open_file_description_mut(fd, |_ofd| { // TODO Implement non-blocking I/O Ok(false) }) }Please ensure that the
get_and_set_nonblocking
method insystem/virtual.rs
is implemented to provide consistent behavior across different system implementations.Analysis chain
Line range hint
139-160
:
Verify the correctness ofget_and_set_nonblocking
integration.Ensure that the new method correctly sets and restores the non-blocking state.
To verify the correctness of the
get_and_set_nonblocking
integration, we need to review the implementation of this method insystem/real.rs
andsystem/virtual.rs
, and ensure that its usage inshared.rs
correctly sets and restores the non-blocking state.Once we have the implementations, we can analyze them to ensure they correctly set and restore the non-blocking state.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of `get_and_set_nonblocking` integration. # Test: Search for the function usage. Expect: Correct integration of the new method. rg --type rust -A 5 $'get_and_set_nonblocking'Length of output: 4577
Script:
#!/bin/bash # Description: Verify the implementation of `get_and_set_nonblocking` in `system/real.rs` and `system/virtual.rs`. # Extract the implementation of `get_and_set_nonblocking` in `system/real.rs`. ast-grep --lang rust --pattern 'fn get_and_set_nonblocking(&mut self, fd: Fd, nonblocking: bool) -> Result<bool> { $$$ }' yash-env/src/system/real.rs # Extract the implementation of `get_and_set_nonblocking` in `system/virtual.rs`. ast-grep --lang rust --pattern 'fn get_and_set_nonblocking(&mut self, fd: Fd, _nonblocking: bool) -> Result<bool> { $$$ }' yash-env/src/system/virtual.rsLength of output: 1750
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
Cargo.lock
is excluded by!**/*.lock
Files selected for processing (22)
- yash-builtin/src/exec.rs (5 hunks)
- yash-builtin/src/source/semantics.rs (2 hunks)
- yash-cli/src/startup/init_file.rs (2 hunks)
- yash-cli/src/startup/input.rs (3 hunks)
- yash-env/CHANGELOG.md (1 hunks)
- yash-env/Cargo.toml (1 hunks)
- yash-env/src/input/fd_reader.rs (2 hunks)
- yash-env/src/lib.rs (1 hunks)
- yash-env/src/system.rs (9 hunks)
- yash-env/src/system/file_system.rs (1 hunks)
- yash-env/src/system/id.rs (2 hunks)
- yash-env/src/system/open_flag.rs (1 hunks)
- yash-env/src/system/real.rs (6 hunks)
- yash-env/src/system/real/open_flag.rs (1 hunks)
- yash-env/src/system/shared.rs (9 hunks)
- yash-env/src/system/virtual.rs (33 hunks)
- yash-env/src/system/virtual/file_system.rs (8 hunks)
- yash-semantics/Cargo.toml (1 hunks)
- yash-semantics/src/command/item.rs (2 hunks)
- yash-semantics/src/command/simple_command/external.rs (7 hunks)
- yash-semantics/src/expansion/glob.rs (2 hunks)
- yash-semantics/src/redir.rs (10 hunks)
Files skipped from review due to trivial changes (1)
- yash-env/src/system/id.rs
Additional comments not posted (84)
yash-semantics/Cargo.toml (1)
18-18
: Addition ofenumset
dependency is appropriate.The
enumset
crate at version1.1.2
is correctly added to handle sets of enum values efficiently. Ensure that this dependency is utilized properly in the codebase.yash-env/Cargo.toml (1)
19-19
: Addition ofbitflags
dependency is appropriate.The
bitflags
crate at version2.6.0
is correctly added to handle sets of flags in a type-safe manner. Ensure that this dependency is utilized properly in the codebase.yash-env/src/system/open_flag.rs (3)
1-20
: License information and imports are appropriate.The file includes the correct GNU General Public License information and imports the
enumset
crate, which is necessary for the functionality provided in this file.
21-35
:OfdAccess
enum is well-defined.The
OfdAccess
enum correctly defines various access modes for open file descriptions, covering all necessary modes.
37-62
:OpenFlag
enum is well-defined and usesEnumSetType
appropriately.The
OpenFlag
enum correctly defines various options for opening file descriptors and uses theEnumSetType
derive macro for efficient handling of enum sets.yash-cli/src/startup/input.rs (3)
37-37
: ImportOfdAccess
andOpenFlag
.The new imports
OfdAccess
andOpenFlag
are necessary for the updated file opening mechanism.
93-93
: Ensure non-blocking state is correctly set.The change to non-blocking reads enhances responsiveness. Ensure that the non-blocking state is correctly set for all relevant file descriptors.
114-119
: Update file opening mechanism.The new file opening mechanism using
OfdAccess::ReadOnly
andOpenFlag::Cloexec
improves semantic clarity. Ensure that the new mechanism is compatible with all intended use cases.yash-env/src/system/file_system.rs (8)
25-28
: LGTM! Constants are correctly defined.The constants
RAW_AT_FDCWD
are correctly defined for Unix and non-Unix platforms.
36-44
: LGTM!DirEntry
struct is correctly defined.The
DirEntry
struct is straightforward and correctly defined.
46-54
: LGTM!Dir
trait is correctly defined.The
Dir
trait is straightforward and correctly defined.
56-72
: DefineRawMode
type alias.The
RawMode
type alias is correctly defined for Unix and non-Unix platforms.
74-82
: DefineMode
struct.The
Mode
struct is correctly defined, implementing the new type pattern for better type safety.
83-128
: DefineMode
bitflags.The
Mode
bitflags are correctly defined, providing a comprehensive set of file permission bits.
130-134
: ImplementDebug
forMode
.The
Debug
implementation forMode
is correctly defined, providing a clear and concise representation.
136-141
: ImplementDefault
forMode
.The
Default
implementation forMode
is correctly defined, setting the default mode to0o644
.yash-env/CHANGELOG.md (3)
12-26
: LGTM!The additions accurately reflect the new types, methods, and dependencies introduced in the codebase.
27-34
: LGTM!The changes accurately reflect the modifications made to the existing types and methods in the codebase.
35-42
: LGTM!The deprecated and removed items accurately reflect the changes made in the codebase.
yash-builtin/src/source/semantics.rs (1)
120-125
: LGTM!The changes to the
open_file
function improve the clarity and type safety by replacingOFlag
withOfdAccess
andOpenFlag
.yash-env/src/input/fd_reader.rs (1)
223-229
: LGTM!The changes to the
open
method improve the granularity of file opening options by replacingOFlag
withOfdAccess
andOpenFlag
.yash-builtin/src/exec.rs (5)
145-145
: Import statement forMode
is appropriate.The import statement is necessary for the new method call to set permissions.
168-168
: Improved permission setting method call.The new method call
content.permissions.set(Mode::USER_EXEC, true)
enhances readability and maintainability.
203-203
: Improved permission setting method call.The new method call
content.permissions.set(Mode::USER_EXEC, true)
enhances readability and maintainability.
238-238
: Improved permission setting method call.The new method call
content.permissions.set(Mode::USER_EXEC, true)
enhances readability and maintainability.
268-268
: Improved permission setting method call.The new method call
content.permissions.set(Mode::USER_EXEC, true)
enhances readability and maintainability.yash-cli/src/startup/init_file.rs (2)
41-41
: Updated import statement is appropriate.The import statement has been updated to replace
OFlag
withOfdAccess
andOpenFlag
, which is necessary for the new types used in theopen_fd
function.
149-154
: Updatedopen_fd
function enhances clarity.The function now uses
OfdAccess::ReadOnly
andOpenFlag::Cloexec.into()
, which enhances the clarity and intent of the code by explicitly defining the access type and flags.yash-semantics/src/command/item.rs (2)
33-33
: Import statement forOfdAccess
is appropriate.The import statement is necessary for the new type used in the
nullify_stdin
function.
119-121
: Updatednullify_stdin
function enhances clarity.The function now uses
OfdAccess::ReadOnly
, which enhances the clarity and intent of the code by explicitly defining the access type.yash-env/src/system/virtual/file_system.rs (6)
33-33
: LGTM! Centralized default directory mode enhances maintainability.The introduction of
DEFAULT_DIRECTORY_MODE
improves code clarity and maintainability by centralizing the definition of default permissions.
50-50
: LGTM! Consistent application of default directory permissions.Using
DEFAULT_DIRECTORY_MODE
in theFileSystem
default implementation ensures consistent application of default permissions across the codebase.
112-112
: LGTM! Adherence to default directory permissions.The
save
method now usesDEFAULT_DIRECTORY_MODE
, ensuring that new directories adhere to the defined default permissions.
155-155
: LGTM! Improved readability of permission check.The permission check in the
get
method now usesnode_ref.permissions.contains(Mode::USER_EXEC)
, improving code readability.
197-223
: LGTM! Enhanced documentation and support for named pipes.The new documentation comments clarify the purpose of each
FileBody
variant, and theFifo
variant adds support for named pipes.
254-257
: LGTM! Promoted consistency in permission handling.The deprecation of the
Mode
struct in favor of a type alias encourages the use ofyash_env::system::Mode
, promoting consistency and maintainability.yash-semantics/src/command/simple_command/external.rs (6)
237-237
: LGTM! Necessary import forMode
type.The import statement for
Mode
has been added to the test module, which is necessary for the test cases to compile and run correctly.
253-253
: LGTM! Improved readability in setting permissions.The test case now uses
content.permissions.set(Mode::USER_EXEC, true)
, improving readability and maintainability.
297-297
: LGTM! Improved readability in setting permissions.The test case now uses
content.permissions.set(Mode::USER_EXEC, true)
, improving readability and maintainability.
323-323
: LGTM! Improved readability in setting permissions.The test case now uses
content.permissions.set(Mode::USER_EXEC, true)
, improving readability and maintainability.
352-352
: LGTM! Improved readability in setting permissions.The test case now uses
content.permissions.set(Mode::USER_EXEC, true)
, improving readability and maintainability.
425-425
: LGTM! Improved readability in setting permissions.The test case now uses
content.permissions.set(Mode::USER_EXEC, true)
, improving readability and maintainability.yash-semantics/src/expansion/glob.rs (1)
297-297
: LGTM! Necessary import forMode
type.The import statement for
Mode
has been added to the test module, which is necessary for the test cases to compile and run correctly.yash-env/src/system/real.rs (4)
274-276
: LGTM!The
ofd_access
function improves specificity and focuses on access control.
279-290
: LGTM!The
get_and_set_nonblocking
function provides explicit control over non-blocking behavior.
351-352
: LGTM!The
umask
function changes ensure that the returned value accurately reflects the new mask set by the call.
231-250
: LGTM! But verify the function usage in the codebase.The refactored
open
function enhances flexibility and type safety.However, ensure that all function calls to
open
match the new signature.Verification successful
LGTM! But verify the function usage in the codebase.
The refactored
open
function enhances flexibility and type safety.However, ensure that all function calls to
open
match the new signature.
All instances of the
open
function match the new signature.The refactored
open
function has been correctly applied across the codebase.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `open` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type rust -A 5 $'open'Length of output: 179215
Script:
#!/bin/bash # Description: Verify all function calls to `open` match the new signature. # Search for the function usage and check for the new signature. rg --type rust -A 5 'open\(' | grep -E 'open\([^,]+, OfdAccess, EnumSet<OpenFlag>, Mode\)'Length of output: 92
Script:
#!/bin/bash # Description: Verify all function calls to `open` match the new signature. # Search for all instances of the open function. rg --type rust 'open\(' -A 5Length of output: 15976
yash-env/src/system.rs (4)
156-157
: LGTM!The
ofd_access
function improves specificity and focuses on access control.
159-164
: LGTM!The
get_and_set_nonblocking
function provides explicit control over non-blocking behavior.
217-217
: LGTM!The
umask
function changes ensure that the returned value accurately reflects the new mask set by the call.
135-141
: LGTM! But verify the function usage in the codebase.The refactored
open
function enhances flexibility and type safety.However, ensure that all function calls to
open
match the new signature.yash-env/src/lib.rs (1)
298-299
: LGTM!The
get_tty
function changes enhance clarity and maintainability by adopting a more explicit representation of access modes.yash-env/src/system/shared.rs (4)
344-345
: Verify the correctness ofget_and_set_nonblocking
integration.Ensure that the new function correctly sets and restores the non-blocking state.
Line range hint
178-207
:
Verify the correctness ofget_and_set_nonblocking
integration.Ensure that the new method correctly sets and restores the non-blocking state.
341-342
: Verify the correctness ofofd_access
integration.Ensure that the new function correctly retrieves the access mode of the file descriptor.
Verification successful
Verify the correctness of
ofd_access
integration.The
ofd_access
function is correctly integrated and retrieves the access mode of the file descriptor as expected in bothreal.rs
andvirtual.rs
.
real.rs
: Usesfcntl
system call to get file status flags.virtual.rs
: Uses internal logic to determine access mode based on readability and writability.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of `ofd_access` integration. # Test: Search for the function usage. Expect: Correct integration of the new function. rg --type rust -A 5 $'ofd_access'Length of output: 2747
Script:
#!/bin/bash # Extract the full implementation of the `ofd_access` function in `real.rs` ast-grep --lang rust --pattern $'fn ofd_access(&self, fd: Fd) -> Result<OfdAccess> { $$$ }' yash-env/src/system/real.rs # Extract the full implementation of the `ofd_access` function in `virtual.rs` ast-grep --lang rust --pattern $'fn ofd_access(&self, fd: Fd) -> Result<OfdAccess> { $$$ }' yash-env/src/system/virtual.rsLength of output: 1894
326-333
: Verify the correctness ofOfdAccess
andEnumSet<OpenFlag>
integration.Ensure that the new parameters are correctly integrated and that the logic remains consistent.
yash-semantics/src/redir.rs (4)
Line range hint
357-394
:
Verify the correctness ofOfdAccess
integration.Ensure that the new parameter is correctly integrated and that the logic remains consistent.
Verification successful
Verify the correctness of
OfdAccess
integration.The
OfdAccess
parameter is correctly integrated into thecopy_fd
function and its usage within the same file. The logic and error handling related toOfdAccess
appear consistent.
copy_fd
function definition:yash-semantics/src/redir.rs
lines 357-394- Function calls with
OfdAccess::ReadOnly
andOfdAccess::WriteOnly
:yash-semantics/src/redir.rs
lines 372-397Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of `OfdAccess` integration. # Test: Search for the function usage. Expect: Correct integration of the new parameter. rg --type rust -A 5 $'copy_fd'Length of output: 804
281-289
: Verify the correctness ofOfdAccess
andEnumSet<OpenFlag>
integration.Ensure that the new parameters are correctly integrated and that the logic remains consistent.
416-431
: Verify the correctness ofOfdAccess
andEnumSet<OpenFlag>
integration.Ensure that the new parameters are correctly integrated and that the logic remains consistent.
Verification successful
Verify the integration of
OfdAccess
andEnumSet<OpenFlag>
inopen_normal
function.Ensure that the new parameters are correctly integrated and that the logic remains consistent.
The integration of
OfdAccess
andEnumSet<OpenFlag>
inopen_normal
and related functions is correct and consistent.
open_file
correctly usesOfdAccess
andEnumSet<OpenFlag>
.open_file_noclobber
andcopy_fd
also utilizeOfdAccess
appropriately.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of `OfdAccess` and `EnumSet<OpenFlag>` integration. # Test: Search for the function usage. Expect: Correct integration of the new parameters. rg --type rust -A 5 $'open_normal'Length of output: 821
Script:
#!/bin/bash # Description: Verify the implementation details of `open_normal` and the functions it calls. # Search for the definition and usage of `open_file`, `open_file_noclobber`, and `copy_fd` functions. rg --type rust -A 5 'fn open_file' rg --type rust -A 5 'fn open_file_noclobber' rg --type rust -A 5 'fn copy_fd'Length of output: 3153
Line range hint
300-323
:
Verify the correctness ofEnumSet<OpenFlag>
integration.Ensure that the new parameter is correctly integrated and that the logic remains consistent.
Verification successful
Verify the correctness of
EnumSet<OpenFlag>
integration.The integration of
EnumSet<OpenFlag>
in the functionopen_file_noclobber
appears correct. The function is only called in one place, and the logic at this call site is consistent with the intended use ofEnumSet<OpenFlag>
.
yash-semantics/src/redir.rs
: Line 313-326Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of `EnumSet<OpenFlag>` integration. # Test: Search for the function usage. Expect: Correct integration of the new parameter. rg --type rust -A 5 $'open_file_noclobber'Length of output: 989
Script:
#!/bin/bash # Description: Search for all instances of `open_file_noclobber` being called in the codebase. # Test: Search for function calls. Expect: Consistent logic and correct integration of `EnumSet<OpenFlag>`. rg --type rust 'open_file_noclobber'Length of output: 271
yash-env/src/system/virtual.rs (24)
28-30
: Documentation Update Approved.The documentation now accurately reflects the support for named pipes and symbolic links in the virtual file system.
66-67
: New Types Imported Approved.The new types
OfdAccess
andOpenFlag
have been imported, which will likely improve type safety and expressiveness.
80-80
: EnumSet Import Approved.The
enumset::EnumSet
type has been imported, which is useful for handling sets of flags.
182-182
: Use of Mode::ALL_9 Approved.The
/tmp
directory is set with full permissions usingMode::ALL_9
, aligning with the new type-safe approach.
324-324
: Use of Mode Type in stat Function Approved.The
stat
function now uses theMode
type to set file permissions, improving clarity and reducing errors.
371-371
: Use of Mode Type in is_executable_file Function Approved.The
is_executable_file
function now uses theMode
type to check for executable permissions, aligning with the new type-safe approach.
440-446
: Updated open Function Signature Approved.The
open
function signature now usesOfdAccess
,EnumSet<OpenFlag>
, andMode
, refining file access mode specifications for better expressiveness and type safety.
451-454
: Open Function Flag Checks Approved.The
open
function now correctly checks for theExclusive
andDirectory
flags using the newOpenFlag
type, adhering to the type-safe approach.
466-469
: Use of Mode Type for Setting File Permissions Approved.The
open
function now sets file permissions using theMode
type when creating a new file, aligning with the type-safe approach.
477-481
: Use of OfdAccess Type in Open Function Approved.The
open
function now uses theOfdAccess
type to determine file access modes, improving clarity and type safety.
501-501
: Use of OpenFlag Type for Setting is_appending Flag Approved.The
open
function now sets theis_appending
flag using theOpenFlag
type, ensuring correct handling of the append mode.
505-505
: Use of OpenFlag Type for Setting flag Field Approved.The
open
function now sets theflag
field using theOpenFlag
type, aligning with the type-safe approach.
538-552
: New ofd_access Function Approved.The
ofd_access
function replaces thefcntl_getfl
function and uses theOfdAccess
type, improving the structured determination of access modes for open file descriptors.
620-622
: Updated opendir Function Approved.The
opendir
function now uses theOfdAccess
andOpenFlag
types, aligning with the type-safe approach.
627-628
: Updated umask Function Signature Approved.The
umask
function signature now uses theMode
type, simplifying the function and aligning with the type-safe approach.
1342-1342
: Updated fstatat Test Approved.The test for
fstatat
now uses theMode
type, ensuring it correctly reflects the new permission handling logic.
1380-1380
: Updated fstatat FIFO Test Approved.The test for
fstatat
with a FIFO now uses theMode
type, ensuring it correctly reflects the new permission handling logic.
1411-1411
: Updated fstatat Symlink to Regular File Test Approved.The test for
fstatat
with a symlink to a regular file now uses theMode
type, ensuring it correctly reflects the new permission handling logic.
1420-1420
: Updated fstatat Symlink No Follow Test Approved.The test for
fstatat
with a symlink and no follow flag now uses theMode
type, ensuring it correctly reflects the new permission handling logic.
1445-1445
: Updated is_executable_file Test Approved.The test for
is_executable_file
with an executable file now uses theMode
type, ensuring it correctly reflects the new permission handling logic.
1527-1529
: Updated Test for Opening Non-Existing File Without Creation Approved.The test for opening a non-existing file without creation now uses the
OfdAccess
andMode
types, ensuring it correctly reflects the new file access and permission handling logic.
1539-1541
: Updated Test for Creating and Opening Non-Existing File Approved.The test for creating and opening a non-existing file now uses the
OfdAccess
andOpenFlag
types, ensuring it correctly reflects the new file access and permission handling logic.
1559-1571
: Updated Test for Opening Existing File Approved.The test for opening an existing file now uses the
OfdAccess
,OpenFlag
, andMode
types, ensuring it correctly reflects the new file access and permission handling logic.
1587-1597
: Updated Test for Opening Existing File with Exclusive Flag Approved.The test for opening an existing file with the
Exclusive
flag now uses
impl OfdAccess { | ||
#[must_use] | ||
pub(super) fn to_real_flags(self) -> Option<c_int> { | ||
match self { | ||
Self::ReadOnly => Some(nix::libc::O_RDONLY), | ||
Self::WriteOnly => Some(nix::libc::O_WRONLY), | ||
Self::ReadWrite => Some(nix::libc::O_RDWR), | ||
// TODO Support O_EXEC, O_PATH and O_SEARCH | ||
Self::Exec | Self::Search => None, | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reminder: Complete support for all flags.
The function to_real_flags
does not support O_EXEC
, O_PATH
, and O_SEARCH
.
Would you like me to assist with implementing support for these flags?
#[must_use] | ||
pub(super) fn from_real_flags(flags: c_int) -> Self { | ||
match flags & nix::libc::O_ACCMODE { | ||
nix::libc::O_RDONLY => Self::ReadOnly, | ||
nix::libc::O_WRONLY => Self::WriteOnly, | ||
nix::libc::O_RDWR => Self::ReadWrite, | ||
_ => Self::Exec, // TODO Support O_PATH and O_SEARCH | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reminder: Complete support for all flags.
The function from_real_flags
does not support O_PATH
and O_SEARCH
.
Would you like me to assist with implementing support for these flags?
fn get_and_set_nonblocking(&mut self, fd: Fd, _nonblocking: bool) -> Result<bool> { | ||
self.with_open_file_description_mut(fd, |_ofd| { | ||
// TODO Implement non-blocking I/O | ||
Ok(false) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New get_and_set_nonblocking Function Approved, but Implementation Needed.
The get_and_set_nonblocking
function has been introduced but currently returns false
. The TODO comment indicates that non-blocking I/O needs to be implemented.
Do you want me to implement the non-blocking I/O functionality or open a GitHub issue to track this task?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- yash-env/src/system/real/open_flag.rs (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- yash-env/src/system/real/open_flag.rs
This is part of #353.
The fcntl_setfl and fcntl_getfl functions are removed in favor of the
new functions get_and_set_nonblocking and ofd_access.
We cannot support the fcntl_setfl function because its return value may
need to contain platform-specific flags that cannot be represented in a
portable type.
The Mode type is replaced with a new homebrew type.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores