winapi suppliment #6338

youknowone · 2025-12-07T14:15:59Z

Summary by CodeRabbit

Release Notes

New Features
- Added GetErrorMode() function for retrieving Windows error mode settings.
- Added Windows reparse point tag constants for enhanced file system operations.
- Added ExitProcess() function for controlled process termination.
Improvements
- Enhanced Windows handle management functions for better system compatibility.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2025-12-07T14:16:04Z

Caution

Review failed

The pull request is closed.

Walkthrough

Three Windows stdlib modules are enhanced with new API bindings: GetErrorMode() function added to msvcrt, three IO_REPARSE_TAG_* constants added to stat under Windows cfg, and winapi.rs receives updated GetStdHandle/DuplicateHandle signatures plus new ExitProcess function.

Changes

Cohort / File(s)	Summary
msvcrt module `crates/vm/src/stdlib/msvcrt.rs`	Adds public Python-exposed `GetErrorMode()` function that wraps Windows Debug::GetErrorMode() API, returning u32 result.
stat module (Windows constants) `crates/vm/src/stdlib/stat.rs`	Adds three Windows-specific reparse point tag constants under `#[cfg(windows)]`: `IO_REPARSE_TAG_SYMLINK`, `IO_REPARSE_TAG_MOUNT_POINT`, `IO_REPARSE_TAG_APPEXECLINK` as public `#[pyattr]` items.
winapi module `crates/vm/src/stdlib/winapi.rs`	Updates `GetStdHandle()` signature to accept `vm: &VirtualMachine` parameter and return `PyResult<Option<HANDLE>>`; refactors `DuplicateHandle()` parameters from tuple `(src_process, src)` to separate `src_process: HANDLE, src: HANDLE` parameters; adds new `ExitProcess(exit_code: u32)` public function.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

Signature changes to GetStdHandle and DuplicateHandle should be checked for call-site compatibility
Verify return type changes in GetStdHandle are correctly handled in existing code paths
Confirm new constants align with official Windows API definitions

Possibly related PRs

RustPython/RustPython#6338: Introduces identical Windows API additions across the same three stdlib modules.
RustPython/RustPython#6018: Also extends Windows-specific constants in stat.rs, providing context for Windows API constant management.

Poem

🐰 Hops through the Windows API ways,
GetErrorMode guides through error maze,
Reparse tags planted in stat's soil,
Exit swift with ExitProcess toil,
Handles duplicated, futures bright! 🪟✨

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'winapi suppliment' is vague and does not clearly convey the scope of changes, which include additions to msvcrt, stat, winapi, codecs, and windows modules.	Consider a more descriptive title that reflects the main changes, such as 'Add Windows API functions and codec support' or 'Extend Windows stdlib modules with new functions and constants'.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% 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 0e0bd1e and a97c075.

⛔ Files ignored due to path filters (3)

Lib/test/test_faulthandler.py is excluded by !Lib/**
Lib/test/test_shutil.py is excluded by !Lib/**
Lib/test/test_subprocess.py is excluded by !Lib/**

📒 Files selected for processing (3)

crates/vm/src/stdlib/msvcrt.rs (1 hunks)
crates/vm/src/stdlib/stat.rs (1 hunks)
crates/vm/src/stdlib/winapi.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.}

coderabbitai

Actionable comments posted: 2

🧹 Nitpick comments (2)

crates/vm/src/stdlib/codecs.rs (2)

239-316: Consider overflow check for large strings (optional).

The implementation correctly handles MBCS encoding with proper surrogate checking, two-pass buffer sizing, and strict error handling. However, the casts from wide.len() to i32 (lines 270, 291) could theoretically overflow for extremely large strings (> 2GB).

This is unlikely in practice and aligns with similar limitations in CPython's implementation.

436-513: Consider overflow check (optional); note code duplication.

The oem_encode implementation is nearly identical to mbcs_encode (differing only in using CP_OEMCP instead of CP_ACP). The same optional consideration applies regarding potential i32 overflow for extremely large strings.

Optional refactor: Consider extracting a common helper function to reduce duplication between mbcs_encode and oem_encode.

📜 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 8781de6 and 0e0bd1e.

⛔ Files ignored due to path filters (3)

Lib/test/test_faulthandler.py is excluded by !Lib/**
Lib/test/test_shutil.py is excluded by !Lib/**
Lib/test/test_subprocess.py is excluded by !Lib/**

📒 Files selected for processing (5)

crates/vm/src/stdlib/codecs.rs (1 hunks)
crates/vm/src/stdlib/msvcrt.rs (1 hunks)
crates/vm/src/stdlib/stat.rs (1 hunks)
crates/vm/src/stdlib/winapi.rs (4 hunks)
crates/vm/src/windows.rs (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (2)

crates/vm/src/stdlib/msvcrt.rs
crates/vm/src/stdlib/stat.rs

🧰 Additional context used

📓 Path-based instructions (1)

**/*.rs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.rs: Follow the default rustfmt code style by running cargo fmt to format Rust code
Always run clippy to lint Rust code (cargo clippy) before completing tasks and fix any warnings or lints introduced by changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust

Files:

crates/vm/src/stdlib/winapi.rs
crates/vm/src/stdlib/codecs.rs
crates/vm/src/windows.rs

🧠 Learnings (1)

📓 Common learnings

Learnt from: moreal
Repo: RustPython/RustPython PR: 5847
File: vm/src/stdlib/stat.rs:547-567
Timestamp: 2025-06-27T14:47:28.810Z
Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration rather than providing fallback values for other platforms.

Learnt from: moreal
Repo: RustPython/RustPython PR: 5847
File: vm/src/stdlib/stat.rs:547-567
Timestamp: 2025-06-27T14:47:28.810Z
Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration using #ifdef checks rather than providing fallback values for other platforms.

🧬 Code graph analysis (2)

crates/vm/src/stdlib/winapi.rs (1)

crates/vm/src/stdlib/os.rs (1)

errno_err (39-41)

crates/vm/src/stdlib/codecs.rs (1)

crates/vm/src/stdlib/nt.rs (1)

from_utf16 (327-329)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)

GitHub Check: Check Rust code with clippy
GitHub Check: Run snippets and cpython tests (windows-latest)
GitHub Check: Run snippets and cpython tests (ubuntu-latest)
GitHub Check: Run snippets and cpython tests (macos-latest)
GitHub Check: Check the WASM package and demo
GitHub Check: Run snippets and cpython tests on wasm-wasi
GitHub Check: Run rust tests (macos-latest)
GitHub Check: Run rust tests (ubuntu-latest)
GitHub Check: Ensure compilation on various targets

🔇 Additional comments (4)

crates/vm/src/windows.rs (1)

172-172: LGTM! Correct symlink file type handling.

The change properly clears existing file type bits using S_IFMT mask before setting the Windows-specific symlink type. This prevents invalid combined file type bits that could result from OR-ing with the previous mode.

crates/vm/src/stdlib/winapi.rs (3)

88-102: LGTM! Proper handle error and null handling.

The refactored GetStdHandle correctly distinguishes between:

INVALID_HANDLE_VALUE → returns error via errno_err(vm)

NULL handle → returns None (e.g., detached console)

Valid handle → returns Some(HANDLE)

This pattern aligns with Rust idioms and provides proper error propagation to Python.

127-128: LGTM! Improved API ergonomics.

Splitting the source handle into two separate parameters (src_process and src) instead of a tuple makes the API more intuitive and idiomatic.

308-311: LGTM! ExitProcess added for process termination.

The function exposes Windows' ExitProcess API, which terminates the process immediately without running destructors or cleanup handlers. This is analogous to Python's os._exit() and is appropriate for the low-level _winapi module.

crates/vm/src/stdlib/codecs.rs

youknowone marked this pull request as ready for review December 7, 2025 14:50

youknowone force-pushed the winapi branch from 8781de6 to 0e0bd1e Compare December 7, 2025 15:45

coderabbitai bot reviewed Dec 7, 2025

View reviewed changes

crates/vm/src/stdlib/codecs.rs Outdated Show resolved Hide resolved

crates/vm/src/stdlib/codecs.rs Outdated Show resolved Hide resolved

youknowone added 5 commits December 8, 2025 01:35

winapi.ExitProcess

54ef8b7

Fix winapi Handle

98c68c4

msvcrt.GetErrorMode

e76fbaa

windows stats

e3f74fa

unmark tests

a97c075

youknowone force-pushed the winapi branch from 0e0bd1e to a97c075 Compare December 7, 2025 16:35

youknowone merged commit f4b8b01 into RustPython:main Dec 7, 2025
12 of 13 checks passed

youknowone deleted the winapi branch December 7, 2025 16:35

This was referenced Dec 7, 2025

windows umask, win32_xstat_slow_impl, fake EXT_SUFFIX #6340

Merged

Replace windows to windows-sys #6356

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

winapi suppliment #6338

winapi suppliment #6338

Uh oh!

youknowone commented Dec 7, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Dec 7, 2025 •

edited

Loading

Review failed

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

winapi suppliment #6338

winapi suppliment #6338

Uh oh!

Conversation

youknowone commented Dec 7, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Release Notes

Uh oh!

coderabbitai bot commented Dec 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review failed

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

youknowone commented Dec 7, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Dec 7, 2025 •

edited

Loading