GV

GeezerVison

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

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Return EBADF for negative stat file descriptors by youknowdot · Pull Request #9008 · RustPython/RustPython

Conversation

@youknowdot

@youknowdot youknowdot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Extract the negative-descriptor fstat guard from #8954 for the Python 3.14 main branch.

os.stat(-5) and os.fstat(-5) can reach rustix with an invalid ordinary descriptor and panic in debug builds. Return EBADF at the shared non-Windows fstat boundary before calling rustix. Add an interpreter regression covering several negative values, including numbers used as directory-descriptor sentinels elsewhere.

This is a four-line implementation change plus one new Rust integration test. Directory-fd parsing and canonical Python tests are unchanged. It is independent of the closerange/termination fixes in #9006.

Validation

Fresh main-based validation on Linux, with source/runtime identity checked:

  • Unpatched main panics on the focused probe and the exact new integration test; the split passes both.
  • Normal commit hooks and strict workspace/C-API Clippy passed.
  • Workspace tests: 1,353 passed, 18 existing ignored. Separate C-API tests: 116 passed, 4 existing ignored.
  • Dev and production-feature release builds passed. Both return EBADF for the tested negative descriptors and preserve valid/closed-fd behavior and six existing supported directory-fd cases.
  • The explicit 22-test canonical selection passed in both builds, with eight unchanged platform/implementation skips. The release test_os fstat selection also passed through cargo run.

The initial broader FileTests run aborts in the separate test_closerange case. The same case aborts on unpatched main; #9006 addresses it. That failed result is retained, and the successful selection excludes that case without changing tests or adding skips. Full local test_os remains unrun because its asyncio setup requires the sandbox-denied AF_UNIX socketpair. Hosted tests remain enabled; cross-platform CI is pending.

The runtime is main's Python 3.14.alpha. Any CPython comparison used the available 3.15.0rc3 interpreter, not a CPython 3.14 build.

AI assistance

Codex assisted with implementation, extraction, source review, regression tests, and automated validation. The original implementation authorship and Assisted-by trailer are preserved; the regression commit also includes its disclosure.

Sorry, something went wrong.

Uh oh!

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

All reactions
youknowdot added 2 commits October 8, 2026 22:46
@youknowdot
Reject negative file descriptors before fstat
2572c59
Return EBADF for negative ordinary descriptors before calling rustix, avoiding debug-build assertions on canonical invalid-fd tests. Keep directory-fd sentinel and absolute-path handling unchanged.

Assisted-by: Codex:model-version-unavailable
@youknowdot
Cover negative stat descriptors through the interpreter
30a1e45
Exercise ordinary negative descriptors including values accepted by rustix as directory sentinels, without changing canonical tests or directory-fd parsing.

Assisted-by: Codex:model-version-unavailable
@coderabbitai

coderabbitai Bot commented Oct 8, 2026 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration
  • Configuration used: Repository: RustPython/RustPython/.coderabbit.yml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 51ffc9a5-7935-494b-89b7-6e1d45a4a7f8

📥 Commits

Reviewing files that changed from the base of the PR and between bb9ef5d and b3052bd.


📒 Files selected for processing (2)
  • crates/host_env/src/fileutils.rs
  • extra_tests/snippets/stdlib_os.py

 ______________________________________________________________________________________________________________________________________________________________________________________________________
< Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it. - Brian Kernighan >
 ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  \
   \   (\__/)
       (•ㅅ•)
       /   づ
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR


  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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
  • X
  • Mastodon
  • Reddit
  • LinkedIn

Comment @coderabbitai help to get the list of available commands.

All reactions

Sorry, something went wrong.

Uh oh!

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

youknowone
youknowone reviewed Oct 8, 2026
View reviewed changes
Comment thread tests/negative_fstat.rs Outdated

@youknowone youknowone Oct 8, 2026

Copy link
Copy Markdown
Member

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.

do not add python test code in rust file

Sorry, something went wrong.

Uh oh!

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

All reactions
@youknowdot
Move negative stat regression into the os snippet
b3052bd
Preserve the original Unix RustPython-only coverage and every regression assertion while removing the redundant Rust interpreter wrapper. CPython uses -1 as a path sentinel, so keep this regression scoped to its original runtime.

Assisted-by: Codex:model-version-unavailable
youknowone
youknowone approved these changes Oct 9, 2026
View reviewed changes
@youknowone
youknowone marked this pull request as ready for review October 9, 2026 09:27
@youknowone
youknowone merged commit 98b2801 into RustPython:main Oct 9, 2026
29 checks passed

Uh oh!

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

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 9, 2026 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-09T09:31:23.925046Z b3052bd Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

All reactions

Sorry, something went wrong.

Uh oh!

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

chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed Oct 9, 2026
View reviewed changes

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b3052bd9c7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread extra_tests/snippets/stdlib_os.py
Comment on lines +16 to +17
for stat in (os.stat, os.fstat):
for fd in (-1, -5, -9, -100, -(2**31)):

@chatgpt-codex-connector chatgpt-codex-connector Bot Oct 9, 2026

Copy link
Copy Markdown

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.

P1 Badge Remove the prohibited test logic

These added loops introduce new test logic and data in extra_tests/snippets/stdlib_os.py, but the root test-code policy says not to modify test assertions, logic, or data and limits acceptable test-file changes to expected-failure decorator updates. Remove this regression-test addition while retaining the Rust fix so the change complies with the repository policy.

AGENTS.md reference: AGENTS.md:L275-L279

Useful? React with 👍 / 👎.

Sorry, something went wrong.

Uh oh!

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

All reactions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chatgpt-codex-connector chatgpt-codex-connector[bot] chatgpt-codex-connector[bot] left review comments
@youknowone youknowone youknowone approved these changes

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.

2 participants

@youknowdot @youknowone
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.