get_inheritable, dup for windows by youknowone · Pull Request #6343 · RustPython/RustPython

youknowone

Caution

Review failed

The pull request is closed.

Walkthrough

Adds Windows CRT-backed file-descriptor operations to the NT stdlib: get_inheritable(fd), dup(fd), and dup2(fd, fd2, inheritable), plus FFI declarations for _dup and _dup2, a Dup2Args struct, and helper logic to set/clear descriptor inheritable state and map errors to Python exceptions.

Changes

Cohort / File(s)	Summary
NT module — FD duplication & inheritance `crates/vm/src/stdlib/nt.rs`	Added `get_inheritable(fd: i32, vm: &VirtualMachine) -> PyResult<bool>`; declared extern C `_dup(fd: i32) -> i32` and `_dup2(fd: i32, fd2: i32) -> i32`; added `dup(fd)` that duplicates an FD and marks the new descriptor non-inheritable; added `Dup2Args` struct and `dup2(args: Dup2Args)` to duplicate onto a target FD with optional `inheritable` control; implemented `close_fd_and_raise` helper and used `raw_set_handle_inheritable` for setting inheritability; exported new functions and struct.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Single-file change with cohesive additions.
Review focus:
- FFI signatures for _dup / _dup2
- Correct handle conversion and raw_set_handle_inheritable usage
- Error handling and resource cleanup in close_fd_and_raise
- Dup2Args pyarg defaults and parsing

Possibly related PRs

Rework crt_fd to be more aligned with io-safety #5789 — Touches the same vm/src/stdlib/nt.rs area and CRT-backed fd/handle inheritance refactor; likely closely related to these additions.

Poem

🐰 I hopped through bytes with a whiskered grin,

I cloned a handle on a midnight spin.
dup and dup2, a neat little trick,
inheritable flags set — click by click.
Carrots for code, and a floppy FD tin.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and concisely identifies the main additions: get_inheritable and dup functions for Windows platform support.
Docstring Coverage	✅ Passed	Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%.

📜 Recent review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 334ef6c and 388bf2b.

⛔ Files ignored due to path filters (2)

Lib/test/test_builtin.py is excluded by !Lib/**
Lib/test/test_os.py is excluded by !Lib/**

📒 Files selected for processing (1)

crates/vm/src/stdlib/nt.rs (4 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}