GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — winapi suppliment by youknowone · Pull Request #6338 · RustPython/RustPython

Conversation

youknowone commented Dec 7, 2025 •

edited by coderabbitai Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

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.}

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

coderabbitai Bot commented Dec 7, 2025 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

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.}

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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

coderabbitai Bot left a comment

Copy link

Copy Markdown

Contributor

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: 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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

crates/vm/src/stdlib/codecs.rs Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

crates/vm/src/stdlib/codecs.rs Resolved

Uh oh!

There was an error while loading. Please reload this page.

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

Uh oh!

There was an error while loading. Please reload this page.

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

Reviewers

coderabbitai[bot]

coderabbitai[bot] left review comments

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

1 participant

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

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

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

youknowone commented Dec 7, 2025 •

edited by coderabbitai Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

coderabbitai Bot commented Dec 7, 2025 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.