Rework crt_fd to be more aligned with io-safety by coolreader18 · Pull Request #5789 · RustPython/RustPython

11-21:

-        let ret = libc::fstat(fd.as_raw(), stat.as_mut_ptr());
+        let ret = libc::fstat(fd.as_raw_fd(), stat.as_mut_ptr());

#!/bin/bash
# list all definitions ending with "fn as_raw"
rg -n "fn as_raw" | head

#!/bin/bash
# Show the impl block for Borrowed<'a> to see which methods it defines
rg -n "impl<'a> Borrowed<'a>" -A15 -B5 common/src/crt_fd.rs

#!/bin/bash
# Locate the struct definition for Borrowed and its impl blocks
rg -n "struct Borrowed" -A5 -B5 common/src/crt_fd.rs
rg -n "impl Borrowed" -A10 -B0 common/src/crt_fd.rs

#!/bin/bash
# Show all `as_raw` method definitions with context
rg -n "pub fn as_raw" -A5 -B5 common/src/crt_fd.rs

No change needed: Borrowed<'_> defines .as_raw(), not .as_raw_fd()
The Borrowed<'_> wrapper in common/src/crt_fd.rs implements:

• pub fn as_raw(&self) -> Raw
• pub fn as_raw(self) -> Raw

It does not expose an as_raw_fd() method. Calling fd.as_raw_fd() will fail to compile, so the existing fd.as_raw() invocation in common/src/fileutils.rs is correct.

Likely an incorrect or invalid review comment.

24-26: Public helpers now take Borrowed<'_> – good upgrade

setmode_binary no longer risks raw-fd misuse. Implementation stays unchanged – looks correct.

75-76: Safety check: extern _setmode relies on Borrowed being #[repr(transparent)]

Passing Borrowed<'_> directly over FFI is fine only if the wrapper is transparent over an int and Copy. Confirm the type definition:

#[repr(transparent)]
#[derive(Copy, Clone)]
pub struct Borrowed<'a>(c::c_int, PhantomData<&'a ()>);

If either attribute is missing, the ABI may break.

103-107: Nice simplification of get_osfhandle error handling

Delegating to crt_fd::as_handle and using IntoPyException keeps the function tight and idiomatic.

335-336: Good use of lifetime parameters for borrowed file descriptors.

The addition of lifetime parameters to structs and functions that handle borrowed file descriptors improves type safety and makes the borrowing relationships explicit. This aligns well with Rust's ownership model.

Also applies to: 341-342, 345-345, 350-350, 412-412, 441-441, 443-443, 582-582, 594-594, 617-617, 896-896, 915-916, 924-926, 938-939, 957-957, 977-980, 983-983, 1208-1209, 1979-1979, 2200-2221

350-350: Consistent API update from raw file descriptors to safe wrappers.

The changes consistently replace raw file descriptor access methods (.get_opt()) with the new API (.raw_opt(), .as_raw(), .as_raw_fd()), which provides better type safety and clearer semantics.

Also applies to: 412-412, 416-417, 594-594, 915-916, 958-958, 1959-1959, 1983-1983

65-66: Consistent lifetime parameter additions for DirFd.

The lifetime parameters added to DirFd usage maintain consistency with the changes in other modules, ensuring proper tracking of borrowed file descriptors.

Also applies to: 71-72, 75-75, 114-115

93-100: Improved type safety for set_inheritable function.

The change from accepting a raw i32 to crt_fd::Borrowed<'_> improves type safety and makes the borrowing relationship explicit. The error handling for the handle conversion is also properly implemented.

100-103: Well-structured lifetime parameter addition to OsPathOrFd.

The addition of the lifetime parameter 'fd to OsPathOrFd properly tracks the lifetime of borrowed file descriptors, maintaining safety invariants across the type system.

Also applies to: 105-105, 119-120, 125-125

149-151: Clean API update for IOErrorBuilder to support lifetime parameters.

The updates to IOErrorBuilder methods to accept impl Into<OsPathOrFd<'a>> provide a flexible and ergonomic API while maintaining proper lifetime tracking.

Also applies to: 154-156, 162-165

4467-4469: Verify crt_fd::close() does not double-drop the descriptor

close() constructs an Owned with from_raw(fd) and passes it by value to
crt_fd::close(). If close() internally closes the FD and then allows
Owned to be dropped (its Drop impl also closes), the descriptor will be
closed twice.

Please confirm the implementation contract:

• If crt_fd::close() consumes the Owned and mem::forgets it, current code
  is fine.
• If it merely forwards to libc::close and returns, the subsequent Drop
  will execute a second close.

Unit-test with dup() plus fcntl(F_GETFD) or audit crt_fd::close().