Skip to content

gh-145335: Fix os functions when passing fd -1 as path#145439

Merged
vstinner merged 5 commits intopython:mainfrom
vstinner:ebadf
Mar 3, 2026
Merged

gh-145335: Fix os functions when passing fd -1 as path#145439
vstinner merged 5 commits intopython:mainfrom
vstinner:ebadf

Conversation

@vstinner
Copy link
Member

@vstinner vstinner commented Mar 2, 2026

os.listdir(-1) and os.scandir(-1) now fail with OSError(errno.EBADF) rather than listing the current directory.

os.listxattr(-1) now fails with OSError(errno.EBADF) rather than listing extended attributes of the current directory.

os.listdir(-1) and os.scandir(-1) now fail with OSError(errno.EBADF)
rather than listing the current directory.

os.listxattr(-1) now fails with OSError(errno.EBADF) rather than
listing extended attributes of the current directory.
@vstinner vstinner added 🔨 test-with-buildbots Test PR w/ buildbots; report in status section and removed 🔨 test-with-buildbots Test PR w/ buildbots; report in status section labels Mar 2, 2026
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @vstinner for commit a894262 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F145439%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Mar 2, 2026
@vstinner
Copy link
Member Author

vstinner commented Mar 3, 2026

Tests failed on 5 buildbots, but failures are unrelated to this change.

buildbot/AMD64 CentOS9 NoGIL Refleaks PR

FAIL: test_free_reference (test.test_concurrent_futures.test_thread_pool.ThreadPoolExecutorTest.test_free_reference)

buildbot/AMD64 Fedora Stable Clang Installed PR
buildbot/PPC64LE Fedora Stable Clang Installed PR
buildbot/s390x Fedora Stable Clang Installed PR
buildbot/x86 Debian Installed with X PR

3 tests failed: test_datetime test_import test_interpreters. It should be fixed by commit 46c5c57.

@vstinner vstinner marked this pull request as ready for review March 3, 2026 10:27
@vstinner
Copy link
Member Author

vstinner commented Mar 3, 2026

The PR is now ready for review.

cc @picnixz @aisk

Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Are there actually other places where we use the path converter and that would be useful? (that is, do you plan to look at other modules as follow-up?)

@vstinner vstinner merged commit 52c8efa into python:main Mar 3, 2026
51 checks passed
@vstinner vstinner deleted the ebadf branch March 3, 2026 12:57
@vstinner
Copy link
Member Author

vstinner commented Mar 3, 2026

Merged. Thanks for the review!

@vstinner
Copy link
Member Author

vstinner commented Mar 3, 2026

@picnixz:

Are there actually other places where we use the path converter and that would be useful? (that is, do you plan to look at other modules as follow-up?)

I had a look and I didn't find other places with the same bug.

  • path_converter() and path_t are specific to posixmodule.c.
  • PyObject_AsFileDescriptor() fails with ValueError if the file descriptor is negative, so check for errors using fd == -1 or fd < 0 is correct.
  • _PyLong_FileDescriptor_Converter() calls PyObject_AsFileDescriptor().
  • Argument Clinic generates PyObject_AsFileDescriptor() calls when using fildes type.
  • open() calls FileIO() which also explicitly rejects negative file descriptors.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants