Define our own Errno #356
Define our own Errno #356
Conversation
WalkthroughThe overarching change across multiple files in the Changes
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 Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (19)
- yash-env/src/input.rs (2 hunks)
- yash-env/src/lib.rs (5 hunks)
- yash-env/src/pwd.rs (3 hunks)
- yash-env/src/semantics.rs (2 hunks)
- yash-env/src/subshell.rs (3 hunks)
- yash-env/src/system.rs (32 hunks)
- yash-env/src/system/errno.rs (1 hunks)
- yash-env/src/system/fd_set.rs (1 hunks)
- yash-env/src/system/real.rs (16 hunks)
- yash-env/src/system/virtual.rs (30 hunks)
- yash-env/src/system/virtual/file_system.rs (6 hunks)
- yash-env/src/system/virtual/io.rs (4 hunks)
- yash-semantics/src/command/compound_command/subshell.rs (1 hunks)
- yash-semantics/src/command/item.rs (1 hunks)
- yash-semantics/src/command/pipeline.rs (4 hunks)
- yash-semantics/src/command/simple_command/absent.rs (1 hunks)
- yash-semantics/src/command/simple_command/external.rs (2 hunks)
- yash-semantics/src/expansion.rs (1 hunks)
- yash-semantics/src/redir.rs (1 hunks)
Additional comments not posted (59)
yash-env/src/system/fd_set.rs (1)
23-23: LGTM! The import change aligns with the PR's objectives to use a customErrnotype.yash-semantics/src/command/simple_command/absent.rs (1)
90-90: LGTM! The change toerrno.to_string()for error messages aligns with the PR's objectives to enhance error handling.yash-env/src/pwd.rs (3)
21-21: LGTM! The addition of theErrnoimport aligns with the PR's objectives to use a customErrnotype.
53-53: LGTM! Changing error handling fromnix::ErrortoErrnoinPreparePwdErroraligns with the PR's objectives.
105-105: LGTM! Updating error mapping fromnix::Error::EILSEQtoErrno::EILSEQis consistent with the PR's goals.yash-semantics/src/command/compound_command/subshell.rs (1)
57-57: LGTM! The change toerrno.to_string()for error messages aligns with the PR's objectives to enhance error handling.yash-env/src/semantics.rs (1)
132-137: LGTM! Updating the error type in theTryFrom<ExitStatus> for Signalimplementation to useErrnoaligns with the PR's objectives.yash-env/src/input.rs (2)
101-101: LGTM! Updating error handling to useErrnoaligns with the PR's objectives.
248-248: LGTM! Adjusting the assertion to useErrno::EBADF.0is consistent with the PR's goals.yash-semantics/src/command/item.rs (1)
92-92: LGTM! The update to useerrno.to_string().into()aligns with Rust idioms and the PR's objectives for improved error handling.yash-env/src/system/errno.rs (1)
1-302: LGTM! TheErrnotype and its associated functionalities are well-defined and align with the PR's objectives for improved error handling.yash-env/src/system/virtual/file_system.rs (3)
21-21: LGTM! The introduction ofErrnoaligns with the PR's objectives for improved error handling.
66-66: LGTM! Updating thesavemethod to useErrnofor error handling is a positive change.
130-130: LGTM! The update to thegetmethod's return type to useErrnois consistent with the PR's objectives.yash-semantics/src/command/simple_command/external.rs (2)
133-133: LGTM! Replacingerrno.desc()witherrno.to_string()standardizes error message formatting.
194-194: LGTM! The update to useerrno.to_string()for error messages enhances consistency.yash-semantics/src/expansion.rs (1)
167-167: LGTM! Usinge.to_string()forCommandSubstErrorimproves error message clarity.yash-env/src/system/real.rs (19)
44-44: Ensure that the aliasNixErrnois used consistently throughout the file to refer tonix::errno::Errno.
80-88: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [83-106]
The
errno_if_m1trait method implementation is concise and correctly uses the newly definedErrnotype for error handling. This is a good use of traits to provide a common functionality across different types.
174-175: The use of the?operator for concise error propagation with the customResulttype is appropriate here.
190-192: Creating a pipe and converting the file descriptors to theFdtype is correctly handled. The use of the?operator simplifies error handling.
204-209: The loop to handleEINTRerrors duringdup2is a good practice for robust error handling in system calls. However, consider adding a brief comment explaining whyEINTRerrors are specifically retried.
218-221: The error handling inopen_tmpfilecorrectly converts a potentialNonefromerror.raw_os_error()to0, aligning with theErrnotype's expectations. This is a good example of defensive programming.
225-230: The loop to handleEINTRandEBADFerrors duringcloseis correctly implemented. The handling ofEBADFby returningOk(())is particularly noteworthy, as it gracefully handles a common error scenario.
255-263: The retry logic forreadin the presence ofEINTRerrors is correctly implemented. This pattern is essential for robust system call handling.
277-288: The conversion ofSeekFromtonix::unistd::Whenceand the subsequent call tolseekare correctly implemented. The use oftry_intofor convertingu64toi64and handling potential overflow withNixErrno::EOVERFLOWis a good practice.
291-299: The handling offdopendirandopendirwithNonNull::new(dir).ok_or_else(NixErrno::last)?is a concise and effective way to handle potential null pointers from these calls.
334-345: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [311-339]
The implementation of
timesandsigmaskmethods correctly uses theErrnotype for error handling and adheres to best practices for interacting with system calls.
383-393: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [342-390]
The
sigactionandkillmethods are correctly implemented, with proper error handling and usage of the customResulttype. The async block inkillis a good use of Rust's async features for potentially blocking operations.
396-402: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [399-409]
The implementation of
selectcorrectly handles the conversion ofOption<&TimeSpec>andOption<&SigSet>to raw pointers, and the use oferrno_if_m1for error handling is consistent with the rest of the file.
424-436: The methodssetpgid,tcgetpgrp, andtcsetpgrpare correctly implemented, with concise error handling using the customResulttype.
460-497: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [446-467]
The
new_child_processandwaitmethods are correctly implemented, showcasing good practices in error handling and process management.
470-475: The retry logic forexecvein the presence ofEINTRerrors is correctly implemented, ensuring robust handling of system calls.
480-486: The methodsgetcwdandchdirare correctly implemented, with concise and effective error handling using the customResulttype.
489-491: The methodgetpwnam_dircorrectly handles potential errors fromnix::unistd::User::from_nameand gracefully converts the result to anOption<std::path::PathBuf>.
539-546: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [494-543]
The
confstr_path,getrlimit, andsetrlimitmethods are correctly implemented, with appropriate error handling and usage of the customResulttype. The handling of platform-specific behavior inconfstr_pathis a good example of conditional compilation.yash-semantics/src/command/pipeline.rs (4)
146-147: The refactoring to construct the error message before callingprint_errorinexecute_job_controlled_pipelineimproves readability and maintainability.
196-197: The refactoring inshift_or_failto construct the error message before callingprint_erroris consistent with the changes inexecute_job_controlled_pipelineand improves code readability.
212-213: Similar to the previous comments, the refactoring inconnect_pipe_and_execute_commandto construct the error message before callingprint_errorenhances the clarity of the error handling logic.
235-235: Simplifying the error message formatting inpid_or_failby directly using theerrnovalue is a good practice, ensuring that the error messages are concise and to the point.yash-env/src/system/virtual/io.rs (2)
113-113: Ensure proper documentation for thereadfunction's behavior with the newErrnoreturn type.
210-210: Verify the handling ofWhence::SeekHoleandWhence::SeekDatain theseekfunction, as they are currently unsupported.Verification successful
To further verify the handling of
Whence::SeekHoleandWhence::SeekDatain theseekfunction, we need to inspect the implementation of theseekfunction directly. Let's proceed with that.
The implementation of the
seekfunction inyash-env/src/system/virtual/io.rsconfirms thatWhence::SeekHoleandWhence::SeekDataare indeed unsupported. For these cases, along with any other unhandledWhencevalues, the function returns anErrno::EINVALerror, aligning with the initial review comment.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Ensure no code paths rely on unsupported Whence::SeekHole or Whence::SeekData. rg "SeekHole|SeekData"Length of output: 108
Script:
#!/bin/bash # Inspect the implementation of the seek function in yash-env/src/system/virtual/io.rs cat yash-env/src/system/virtual/io.rs | grep -A 20 "pub fn seek"Length of output: 939
yash-env/src/lib.rs (2)
336-336: Add error handling documentation forwait_for_subshellto clarify expectedErrnovalues.
360-360: Consider implementing a timeout mechanism forwait_for_subshell_to_finishto prevent indefinite waiting.yash-env/src/subshell.rs (3)
33-33: Import ofErrnoaligns with the PR's objective to utilize the customErrnotype for error handling.
154-154: The update to thestartfunction's signature to use the customErrnotype improves error handling and type safety.
242-242: The update to thestart_and_waitfunction's signature to use the customErrnotype is consistent with the PR's objectives, enhancing error handling and type safety.yash-semantics/src/redir.rs (6)
194-194: Consider providing more descriptive error messages forFdNotOverwrittenerrors.The current implementation directly converts the
errnoto a string for the error message. While this is technically correct, it might not provide enough context for users to understand the error. Consider appending a more descriptive message before theerrnostring to clarify the nature of the error.
196-196: Ensure consistent error message formatting forOpenFileerrors.The error message for
OpenFileerrors combines the path and theerrnodirectly. This is consistent with the other changes, but consider if additional context or formatting could make these messages clearer or more user-friendly.
200-200: Use descriptive error messages forTemporaryFileUnavailableerrors.Similar to the
FdNotOverwrittenerror, the message forTemporaryFileUnavailableerrors directly uses theerrnostring. It might be beneficial to prepend a descriptive message to provide more context about the error, especially since temporary file creation issues can stem from various system limitations or configurations.
Line range hint
1-1: Verify the license compatibility and adherence.Ensure that the use of the GNU General Public License (GPL) for this file is compatible with the rest of the project's licensing and that it adheres to the GPL's requirements. This is especially important if the project is intended to be used as a library or incorporated into other software.
Line range hint
1-1: Consider adding module-level documentation.The file lacks module-level documentation that explains its purpose and how it fits into the larger project. Adding a brief overview at the beginning of the file could improve maintainability and help new contributors understand the role of this module in the context of the project.
Line range hint
1-1: Evaluate the use ofasyncin file operations.The use of
asyncfunctions for file operations, such asopen_normal, is interesting but requires careful consideration. Ensure that the asynchronous operations are truly beneficial and do not introduce unnecessary complexity or performance overhead, especially since file I/O can often be blocking by nature.yash-env/src/system.rs (1)
118-171: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [94-1004]
Updated function signatures to use the local
Resulttype instead ofnix::Result. This change is crucial for integrating the customErrnotype across the system's API, enhancing type safety and error handling consistency. Verify that all instances wherenix::Resultwas previously used have been updated accordingly and that error handling logic has been adapted to work with the newErrnotype.yash-env/src/system/virtual.rs (5)
67-67: Consider documenting the purpose of theResulttype alias within the context ofVirtualSystem.Adding a comment explaining that
Resultis a custom result type specific to the virtual system's error handling could improve code readability and maintainability.
219-221: Ensure proper documentation for the closure parameterFinwith_open_file_description.Adding a brief comment describing the expected behavior or purpose of the closure
Fcan enhance code readability and maintainability, especially for complex closures.
232-234: Similar to the previous comment, ensure proper documentation for the closure parameterFinwith_open_file_description_mut.
1256-1256: Ensure theExecutortrait'sspawnmethod is documented, especially regarding its return values.Documenting the expected outcomes of the
spawnmethod, including potential errors, can aid in understanding and using this trait effectively.
930-930: Clarify the TODO comment in thewaitmethod regarding supportedtargetvalues.Expanding on the TODO comment to specify what needs to be implemented or changed for unsupported
targetvalues can guide future development efforts.
| mod errno; | ||
| pub mod fd_set; | ||
| pub mod real; | ||
| pub mod resource; | ||
| pub mod r#virtual; | ||
|
|
||
| pub use self::errno::Errno; | ||
| pub use self::errno::RawErrno; | ||
| pub use self::errno::Result; |
There was a problem hiding this comment.
Introduced errno module and re-exported Errno, RawErrno, and Result types. This aligns with the PR's objective to enhance error handling and type safety by encapsulating errno values within a custom type. Ensure that the Errno type and its associated functionalities are thoroughly tested, especially the conversions between Errno and other error types like std::io::Error.
|
|
||
| fn reset_nonblocking(&mut self, fd: Fd, old_flags: OFlag) { | ||
| if !old_flags.contains(OFlag::O_NONBLOCK) { | ||
| let _: Result<(), _> = self.0.borrow_mut().system.fcntl_setfl(fd, old_flags); | ||
| let _: Result<()> = self.0.borrow_mut().system.fcntl_setfl(fd, old_flags); | ||
| } | ||
| } | ||
|
|
||
| /// Reads from the file descriptor. | ||
| /// | ||
| /// This function waits for one or more bytes to be available for reading. | ||
| /// If successful, returns the number of bytes read. | ||
| pub async fn read_async(&mut self, fd: Fd, buffer: &mut [u8]) -> nix::Result<usize> { | ||
| pub async fn read_async(&mut self, fd: Fd, buffer: &mut [u8]) -> Result<usize> { | ||
| let flags = self.set_nonblocking(fd)?; | ||
|
|
||
| // We need to retain a strong reference to the waker outside the poll_fn |
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [692-746]
Implemented asynchronous read and write operations in SharedSystem using non-blocking I/O and futures. This approach is essential for supporting concurrent I/O in an async function context. Ensure that error handling in these asynchronous operations correctly handles EAGAIN and EINTR errors by retrying the operation, and that the non-blocking flag is properly managed to avoid altering the expected behavior of file descriptors outside these operations.
| } | ||
|
|
||
| /// Calls `sigmask` and updates `self.wait_mask`. | ||
| fn sigmask(&mut self, how: SigmaskHow, signal: Signal) -> nix::Result<()> { | ||
| fn sigmask(&mut self, how: SigmaskHow, signal: Signal) -> Result<()> { | ||
| let mut set = SigSet::empty(); | ||
| let mut old_set = SigSet::empty(); | ||
| set.add(signal); |
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1026-1093]
Enhanced signal handling with set_signal_handling and related functions. This change is part of the effort to improve the system's responsiveness and reliability by properly managing signal handling in an asynchronous context. Ensure that signal handling setup, including blocking and unblocking signals and setting signal actions, is correctly implemented to avoid race conditions or missed signals. Additionally, verify that the signal handling changes do not interfere with the system's existing signal handling mechanisms outside of SharedSystem.
| // TODO Need nix 0.28.0 | ||
| // /// Sets the current `errno` value. | ||
| // /// | ||
| // /// This function sets the current `errno` value to the specified value. | ||
| // /// The next call to [`last`](Self::last) will return the specified value | ||
| // /// unless another system call changes the `errno` value. This function is | ||
| // /// useful when you want to simulate an error condition in a system call. | ||
| // /// | ||
| // /// Use [`clear`](Self::clear) to reset the `errno` value. | ||
| // pub fn set_last(errno: Self) { | ||
| // nix::Error::set_raw(errno.0) | ||
| // } |
There was a problem hiding this comment.
Consider tracking the implementation of set_last() method with a GitHub issue once the nix dependency is updated to version 0.28.0.
Would you like me to open a GitHub issue for this?
| @@ -251,7 +252,7 @@ impl VirtualSystem { | |||
| _dir_fd: Fd, | |||
| path: &Path, | |||
| flags: AtFlags, | |||
| ) -> nix::Result<Rc<RefCell<INode>>> { | |||
| ) -> Result<Rc<RefCell<INode>>> { | |||
There was a problem hiding this comment.
Handle TODO for resolving relative paths based on dir_fd.
This TODO indicates a missing feature that could affect the correctness of path resolution in certain scenarios. Would you like assistance in implementing this feature?
| @@ -541,37 +542,37 @@ | |||
| } | |||
|
|
|||
| /// Current implementation does nothing but return `Ok(())`. | |||
| fn fcntl_setfl(&mut self, _fd: Fd, _flags: OFlag) -> nix::Result<()> { | |||
| fn fcntl_setfl(&mut self, _fd: Fd, _flags: OFlag) -> Result<()> { | |||
There was a problem hiding this comment.
Address the TODO in fcntl_setfl to implement the function's intended behavior.
Leaving this function unimplemented might lead to unexpected behavior in tests relying on file descriptor flags being set correctly. If needed, I can help with the implementation.
| @@ -156,7 +156,7 @@ impl OpenFileDescription { | |||
| /// Writes to this open file description. | |||
| /// | |||
| /// Returns the number of bytes successfully written. | |||
| pub fn write(&mut self, mut buffer: &[u8]) -> nix::Result<usize> { | |||
| pub fn write(&mut self, mut buffer: &[u8]) -> Result<usize, Errno> { | |||
There was a problem hiding this comment.
Consider adding unit tests for the write function to cover scenarios with different Errno values.
Would you like me to help by adding some unit tests for this function?
There was a problem hiding this comment.
Actionable comments posted: 3
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (19)
- yash-env/src/input.rs (2 hunks)
- yash-env/src/lib.rs (5 hunks)
- yash-env/src/pwd.rs (3 hunks)
- yash-env/src/semantics.rs (2 hunks)
- yash-env/src/subshell.rs (3 hunks)
- yash-env/src/system.rs (32 hunks)
- yash-env/src/system/errno.rs (1 hunks)
- yash-env/src/system/fd_set.rs (1 hunks)
- yash-env/src/system/real.rs (16 hunks)
- yash-env/src/system/virtual.rs (30 hunks)
- yash-env/src/system/virtual/file_system.rs (6 hunks)
- yash-env/src/system/virtual/io.rs (4 hunks)
- yash-semantics/src/command/compound_command/subshell.rs (1 hunks)
- yash-semantics/src/command/item.rs (1 hunks)
- yash-semantics/src/command/pipeline.rs (4 hunks)
- yash-semantics/src/command/simple_command/absent.rs (1 hunks)
- yash-semantics/src/command/simple_command/external.rs (2 hunks)
- yash-semantics/src/expansion.rs (1 hunks)
- yash-semantics/src/redir.rs (1 hunks)
Files skipped from review as they are similar to previous changes (18)
- yash-env/src/input.rs
- yash-env/src/lib.rs
- yash-env/src/pwd.rs
- yash-env/src/semantics.rs
- yash-env/src/subshell.rs
- yash-env/src/system.rs
- yash-env/src/system/errno.rs
- yash-env/src/system/fd_set.rs
- yash-env/src/system/virtual.rs
- yash-env/src/system/virtual/file_system.rs
- yash-env/src/system/virtual/io.rs
- yash-semantics/src/command/compound_command/subshell.rs
- yash-semantics/src/command/item.rs
- yash-semantics/src/command/pipeline.rs
- yash-semantics/src/command/simple_command/absent.rs
- yash-semantics/src/command/simple_command/external.rs
- yash-semantics/src/expansion.rs
- yash-semantics/src/redir.rs
Additional comments not posted (13)
yash-env/src/system/real.rs (13)
44-44: The import ofnix::errno::Errno as NixErrnois necessary due to the selective retention ofnix::errno::Errnofor specific use cases. Ensure that this import is used consistently throughout the file.
80-88: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [83-106]
The
errno_if_m1trait method correctly converts a result of-1to anErrorwith the currenterrno, leveraging the newly introducedErrnotype. This is a good use of Rust's traits to provide a common functionality across different numeric types.
208-209: The handling ofNixErrno::EINTRin a loop for retrying system calls is a best practice for dealing with interruptions. This pattern is correctly applied in thedup2method.
221-221: When handling errors inopen_tmpfile, ensure thatErrno(error.raw_os_error().unwrap_or(0))is the best way to convert the error. Consider if there's a more descriptive default error than0in caseraw_os_error()returnsNone.
228-230: The handling ofNixErrno::EBADFandNixErrno::EINTRin theclosemethod is appropriate. ReturningOk(())forEBADFmight be acceptable depending on the project's error handling strategy, but ensure this behavior is documented.
259-272: The retry logic forreadandwritemethods in the presence ofNixErrno::EINTRis correctly implemented. This ensures robustness in system call handling.
281-281: Inlseek, converting theoffsettoc_intand handlingEOVERFLOWis a good practice to ensure the call's safety. This prevents potential issues with large offsets.
293-299: The use ofNonNull::new(dir).ok_or_else(NixErrno::last)?infdopendirandopendirmethods is a concise way to handle potential null pointers from C library calls.
337-339: The implementation ofsigmaskusingnix::sys::signal::sigprocmaskdirectly and returning a simpleOk(())upon success is straightforward and clear.
387-390: The use ofasyncblock inkillmethod is an interesting approach to potentially blocking operations. Ensure that this fits well with the overall asynchronous design of the system.Verification successful
The script output indicates that the use of
async moveblocks is a common pattern within theyash-env/src/systemmodule, suggesting that the asynchronous design is consistently applied across the system. This supports the notion that the use of anasync moveblock in thekillmethod aligns with the overall asynchronous design of the system.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the usage of async blocks in potentially blocking operations across the project. rg --type rust 'async move' yash-env/src/systemLength of output: 1087
409-409: Theerrno_if_m1method is correctly used inselectto handle-1results, showcasing the utility of theErrnoIfM1trait for error handling in system calls.
542-543: The use oferrno_if_m1insetrlimitto handle potential errors is consistent with the rest of the file. This maintains the uniformity in error handling.
560-565: The implementation ofnextinRealDircorrectly handles directory reading and error checking. The use ofErrno::clear()before the call and checkingErrno::last()after is a good practice.
| @@ -30,6 +30,7 @@ use super::FdSet; | |||
| use super::FileStat; | |||
| use super::Mode; | |||
| use super::OFlag; | |||
| use super::Result; | |||
There was a problem hiding this comment.
Group use statements for better readability.
use super::{Result, SigSet, SigmaskHow, Signal, System, TimeSpec, Times};| Err(Errno::EINTR) => (), | ||
| other => return other, | ||
| Err(NixErrno::EBADF) => return Ok(()), | ||
| Err(NixErrno::EINTR) => (), | ||
| other => return Ok(other?), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| fn fcntl_getfl(&self, fd: Fd) -> nix::Result<OFlag> { | ||
| nix::fcntl::fcntl(fd.0, nix::fcntl::FcntlArg::F_GETFL).map(OFlag::from_bits_truncate) | ||
| fn fcntl_getfl(&self, fd: Fd) -> Result<OFlag> { | ||
| let bits = nix::fcntl::fcntl(fd.0, nix::fcntl::FcntlArg::F_GETFL)?; | ||
| Ok(OFlag::from_bits_truncate(bits)) | ||
| } | ||
|
|
||
| fn fcntl_setfl(&mut self, fd: Fd, flags: OFlag) -> nix::Result<()> { | ||
| nix::fcntl::fcntl(fd.0, nix::fcntl::FcntlArg::F_SETFL(flags)).map(drop) | ||
| fn fcntl_setfl(&mut self, fd: Fd, flags: OFlag) -> Result<()> { | ||
| let _ = nix::fcntl::fcntl(fd.0, nix::fcntl::FcntlArg::F_SETFL(flags))?; | ||
| Ok(()) | ||
| } | ||
|
|
||
| fn fcntl_getfd(&self, fd: Fd) -> nix::Result<FdFlag> { | ||
| nix::fcntl::fcntl(fd.0, nix::fcntl::FcntlArg::F_GETFD).map(FdFlag::from_bits_truncate) | ||
| fn fcntl_getfd(&self, fd: Fd) -> Result<FdFlag> { | ||
| let bits = nix::fcntl::fcntl(fd.0, nix::fcntl::FcntlArg::F_GETFD)?; | ||
| Ok(FdFlag::from_bits_truncate(bits)) | ||
| } | ||
|
|
||
| fn fcntl_setfd(&mut self, fd: Fd, flags: FdFlag) -> nix::Result<()> { | ||
| nix::fcntl::fcntl(fd.0, nix::fcntl::FcntlArg::F_SETFD(flags)).map(drop) | ||
| fn fcntl_setfd(&mut self, fd: Fd, flags: FdFlag) -> Result<()> { | ||
| let _ = nix::fcntl::fcntl(fd.0, nix::fcntl::FcntlArg::F_SETFD(flags))?; | ||
| Ok(()) | ||
| } | ||
|
|
||
| fn isatty(&self, fd: Fd) -> nix::Result<bool> { | ||
| nix::unistd::isatty(fd.0) | ||
| fn isatty(&self, fd: Fd) -> Result<bool> { | ||
| Ok(nix::unistd::isatty(fd.0)?) | ||
| } | ||
|
|
||
| fn read(&mut self, fd: Fd, buffer: &mut [u8]) -> nix::Result<usize> { | ||
| fn read(&mut self, fd: Fd, buffer: &mut [u8]) -> Result<usize> { | ||
| loop { | ||
| let result = nix::unistd::read(fd.0, buffer); | ||
| if result != Err(Errno::EINTR) { | ||
| return result; | ||
| if result != Err(NixErrno::EINTR) { | ||
| return Ok(result?); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| fn write(&mut self, fd: Fd, buffer: &[u8]) -> nix::Result<usize> { | ||
| fn write(&mut self, fd: Fd, buffer: &[u8]) -> Result<usize> { | ||
| loop { | ||
| let result = nix::unistd::write(fd.0, buffer); | ||
| if result != Err(Errno::EINTR) { | ||
| return result; | ||
| if result != Err(NixErrno::EINTR) { | ||
| return Ok(result?); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| fn lseek(&mut self, fd: Fd, position: SeekFrom) -> nix::Result<u64> { | ||
| fn lseek(&mut self, fd: Fd, position: SeekFrom) -> Result<u64> { | ||
| use nix::unistd::Whence::*; | ||
| let (offset, whence) = match position { | ||
| SeekFrom::Start(offset) => { | ||
| let offset = offset.try_into().map_err(|_| Errno::EOVERFLOW)?; | ||
| let offset = offset.try_into().map_err(|_| NixErrno::EOVERFLOW)?; | ||
| (offset, SeekSet) | ||
| } | ||
| SeekFrom::End(offset) => (offset, SeekEnd), | ||
| SeekFrom::Current(offset) => (offset, SeekCur), | ||
| }; | ||
| nix::unistd::lseek(fd.0, offset, whence).map(|new_offset| new_offset as u64) | ||
| let new_offset = nix::unistd::lseek(fd.0, offset, whence)?; | ||
| Ok(new_offset as u64) | ||
| } | ||
|
|
||
| fn fdopendir(&mut self, fd: Fd) -> nix::Result<Box<dyn Dir>> { | ||
| fn fdopendir(&mut self, fd: Fd) -> Result<Box<dyn Dir>> { | ||
| let dir = unsafe { nix::libc::fdopendir(fd.0) }; | ||
| let dir = NonNull::new(dir).ok_or_else(Errno::last)?; | ||
| let dir = NonNull::new(dir).ok_or_else(NixErrno::last)?; | ||
| Ok(Box::new(RealDir(dir))) | ||
| } | ||
|
|
||
| fn opendir(&mut self, path: &CStr) -> nix::Result<Box<dyn Dir>> { | ||
| fn opendir(&mut self, path: &CStr) -> Result<Box<dyn Dir>> { | ||
| let dir = unsafe { nix::libc::opendir(path.as_ptr()) }; | ||
| let dir = NonNull::new(dir).ok_or_else(Errno::last)?; | ||
| let dir = NonNull::new(dir).ok_or_else(NixErrno::last)?; | ||
| Ok(Box::new(RealDir(dir))) | ||
| } | ||
|
|
There was a problem hiding this comment.
Tip
Codebase Verification
The verification process has identified that std::io::Result is still being used in the getrlimit and setrlimit functions within the yash-env/src/system/real.rs file, which contradicts the requirement to use the project's Result type exclusively for system calls. This confirms the validity of the review comment.
Analysis chain
📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [174-252]
Ensure that all system calls now correctly use the project's Result type instead of std::io::Result or nix::Result. This change is crucial for maintaining consistency in error handling across the project.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify that `std::io::Result` and `nix::Result` are no longer used in favor of the project's `Result` type.
rg --type rust 'std::io::Result|nix::Result' yash-env/src/system/real.rs
Length of output: 242
This PR is part of #353.
This PR introduces the
Errnotype that wraps a rawerrnovalue. This new type replaces all uses ofnix::errno::Errnoexcept in the following implementations:Errnotype,RealSystem.