gh-157899: Add support for fds pointing to symlinks in os.readlink(). by OOTS · Pull Request #157915 · python/cpython

OOTS · 2026-09-21T14:35:18Z

See issue #157899 and https://discuss.python.org/t/support-file-descriptors-in-os-readlink/109101/6 for context.

Issue: Support file descriptors in os.readlink() #157899

python-cla-bot · 2026-09-21T14:35:23Z

All commit authors signed the Contributor License Agreement.

OOTS · 2026-09-21T14:36:43Z

   <dir_fd>`.

+   On MacOS and Linux, *path* can also be a file descriptor referring to a
+   symbolic link. In that case, *dir_fd* must be ``None``, and the return


Another sensible behaviour would be to ignore dir_fd completely, but I think raising an error here is the better alternative.

OOTS · 2026-09-21T14:37:20Z


+   On MacOS and Linux, *path* can also be a file descriptor referring to a
+   symbolic link. In that case, *dir_fd* must be ``None``, and the return
+   value will be a ``bytes`` object.


This just so happened - maybe a str is more reasonable?

Yes, readlins returns str (with surogatepass encoding, so you can get the bytes out losslessly).

Updated: The implementation returns a "string" (via PyUnicode_DecodeFSDefaultAndSize), the tests check for equality with a string ("symlink" rather than b"symlink"), and the docs are updated, too.

OOTS · 2026-09-21T14:38:53Z

+    int use_freadlink = 0;
+#endif
+
+    if (path_and_dir_fd_invalid("readlink", path, dir_fd)) {


The error message here isn't super descriptive - but I figured it was easier to just re-use the existing function. Let me know if you want a change.

read-the-docs-community · 2026-09-21T14:39:08Z

Documentation build overview

📚 cpython-previews | 🛠️ Build #34786052 | 📁 Comparing 7b566b7 against main (a6ad448)

🔍 Preview build

29 files changed · + 1 added · ± 28 modified

+ Added

library/dialog.html

± Modified

OOTS · 2026-09-21T14:44:25Z

+        with self.prepare_file() as (dir_fd, name, fullname):
+            fd = os.open(fullname, os.O_RDONLY)
+            self.addCleanup(os.close, fd)
+            with self.assertRaises(OSError):


My AI was obsessing a bit about this: Depending on circumstances (how you create the FD and the OS) it raises a bare OSError (due to EINVAL), or FileNotFoundError (due to ENOENT).

I agree that consistency would be nice, but I thought tinkering with the error handling is probably worse than this little inconsistency.

Can you put this comment in the test code?

Added a few comment lines into the test.

OOTS · 2026-09-21T14:46:24Z

One open question is whether os.readlink() should be added to supports_fd. I don't have an opinion on that, happy to take others' lead here.

…ink(). This feature is only supported on Linux, Android and MacOS, other OSs should raise a NotImplementedError().

…e carefully.

…_FREADLINK.

encukou · 2026-09-24T12:01:42Z


+   On MacOS and Linux, *path* can also be a file descriptor referring to a
+   symbolic link. In that case, *dir_fd* must be ``None``, and the return
+   value will be a ``bytes`` object.


Yes, readlins returns str (with surogatepass encoding, so you can get the bytes out losslessly).

encukou · 2026-09-24T12:04:07Z

+    }
+
+    if (path->is_fd) {
+#if defined(__APPLE__) && defined(_Py_HAVE_FREADLINK)


I think we only want _Py_HAVE_FREADLINK here -- if any other platform has it, it's the function to call.

Removed the defined(__APPLE__) check.

encukou · 2026-09-24T12:12:09Z

   <dir_fd>`.

+   On MacOS and Linux, *path* can also be a file descriptor referring to a
+   symbolic link. In that case, *dir_fd* must be ``None``, and the return


It would be nice to document how to get such a file descriptor (O_SYMLINK and O_PATH|O_NOFOLLOW).

Added this to the docs.

encukou · 2026-09-24T12:17:43Z

    int readlinkat_unavailable = 0;
 #endif
+#ifdef _Py_HAVE_FREADLINK
+    int freadlink_unavailable = 0;


AFAICS, we don't need the *_unavailable variables. The pattern is sometimes used because we can't raise exceptions inside Py_BEGIN_ALLOW_THREADS/Py_END_ALLOW_THREADS, but here, moving the Py_BEGIN_ALLOW_THREADS/Py_END_ALLOW_THREADS to tightly wrap each freadlink/readlinkat/readlink call would allow setting the exception & returning direclty in the else blocks.

Done. I dropped the freadlink_unavailable variable and the error is now raised directly in the else branch.

I also "inlined" all the invocations of Py_BEGIN_ALLOW_THREADS and Py_END_ALLOW_THREADS, each syscall (freadlink, readlinkat in the path->is_fd case, readlinkat in the old dir_fd case, readlink) are now directly surrounded by those macros.

This is technically a bit of a refactor and slightly out-of-scope of the issue - but I hope it's okay to piggy-back this refactor onto this MR.

encukou · 2026-09-24T12:22:47Z

+        PyErr_SetString(PyExc_NotImplementedError,
+            "readlink cannot read file descriptors on this platform");
+        return NULL;


I don't see the reason for having this twice (it's also on line 11113).

The one below is in the windows branch. This one is inside the HAVE_READLINK branch of this function.

encukou · 2026-09-24T12:26:13Z

+        // Linux: readlinkat(dir_fd, "", ...) reads the symbolic link
+        // pointed to by dir_fd
+        dir_fd = path->fd;
+        path->narrow = "";


Putting the values in these variables is misleading.
Instead, could you add a separate block below with a call to readlinkat(path->fd, "", ...) & the error handling?

Done. We now have a if (path->is_fd) { ... } else if (path->dir_fd != DEFAULT_DIR_FD) { ... } else { ... } structure.

The different implementations of this feature (freadlink on MacOS, readlinkat on Linux/Android, NotImplementedError on Android) are now all inside the if (path->is_fd) { ... } branch.

encukou · 2026-09-24T12:27:17Z

+        with self.prepare_file() as (dir_fd, name, fullname):
+            fd = os.open(fullname, os.O_RDONLY)
+            self.addCleanup(os.close, fd)
+            with self.assertRaises(OSError):


Can you put this comment in the test code?

encukou · 2026-09-24T12:31:53Z

+    if (path->is_fd) {
+#if defined(__APPLE__) && defined(_Py_HAVE_FREADLINK)
+        /* nop, freadlink is called below */
+#elif defined(__linux__) && defined(HAVE_READLINKAT)


I think it would be useful to make this a configure variable, something like _Py_READLINKAT_SUPPORTS_EMPTY_PATH, only set on Linux.
Then, when someone ports CPython to another system that uses this (or to ancient Linux that doesn't), they can just change pyconfig.h or the configure script.

Alternately, use defined(HAVE_READLINKAT) && defined(O_PATH). O_PATH and empty pathname support were added in Linux 2.6.39; they look like it's essentially a single feature.

I tried with defined(HAVE_READLINKAT) && defined(O_PATH), but that broke on the emscripten build. (Apparently both Macros are defined there even though readlink is not supported on that platform AFAIK.)

So now _Py_READLINKAT_SUPPORTS_EMPTY_PATH is set automatically on Linux and Android (that have O_PATH) above. Anyone who really wants it can still force that variable.

I didn't add anything about this to the autoconfig part - I hope that's okay. If you want it threaded through/into the configure.ac somehow, I can look into that.

encukou · 2026-09-24T12:46:09Z

  (Contributed by Md Arif in :gh:`152936`.)

+* :func:`os.readlink` now accepts a file descriptor referring to a symlink on
+  Linux, Android and MacOS.


Could you add Android to the other docs?

encukou · 2026-09-24T12:53:15Z

+    _support_readlink_with_fd = hasattr(os, 'readlink') and (
+        "HAVE_FREADLINK" in posix._have_functions  # MacOS
+        or (
+            os.readlink in os.supports_dir_fd
+            and sys.platform in ["linux", "android"]
+        )
+    )


One open question is whether os.readlink() should be added to supports_fd. I don't have an opinion on that, happy to take others' lead here.

I'd say add them. O_SYMLINK/O_PATH|O_NOFOLLOW are the way to get fds referring to a symlink on their respective platform, so this can be just os.readlink in os.supports_dir_fd.
(Yes, I changed my mind after going through the code.)

Then, please also test that os.readlink in os.supports_dir_fd holds on MacOS, Linux & Android.

Done. os.readlink is now added automatically to os.supports_fd. I also added a test that checks presence/absence on all platforms where both os.readlink and os.supports_fd are defined.

…ddress feedback from MR. * os.readlink() now returns a `str` (rather than `bytes`) when called with a file descriptor. * os.readlink now added to `os.supports_fd` on the platforms where this feature is supported. * Small refactor inside `os.readlink`: drop *_unavailable variables, generate error messages inline. Py_BEGIN_ALLOW_THREADS/Py_END_ALLOW_THREADS around each syscall site. * Docs: Android is now mentioned as supported platform for this feature in all relevant places.

Emscripten apparrently defines O_PATH and readlinkat - even though it doesn't actually support them. This broke auto-detection of whether os.readlink supports FDs - which broke the tests.

OOTS · 2026-09-27T13:12:17Z

Thank you for your review @encukou . I updated this PR based on your comments.

Please re-review when you can spare some minutes.

bedevere-app Bot mentioned this pull request Sep 21, 2026

Support file descriptors in os.readlink() #157899

Open

OOTS commented Sep 21, 2026

View reviewed changes

OOTS marked this pull request as ready for review September 21, 2026 19:45

OOTS requested review from AA-Turner, corona10, emmatyping, erlend-aasland and itamaro as code owners September 21, 2026 19:45

bedevere-app Bot added the awaiting review label Sep 21, 2026

OOTS force-pushed the feature/gh-157899-fds-in-os-readlink branch from d6a5c5c to d7fa0ad Compare September 23, 2026 08:12

OOTS added 3 commits September 23, 2026 11:25

pythongh-157899: Add support for fds pointing to symlinks in os.readl…

2cbd30e

…ink(). This feature is only supported on Linux, Android and MacOS, other OSs should raise a NotImplementedError().

fix compiler error on MacOS. Calls to freadlink must be guarded mor…

8bb6f09

…e carefully.

pythongh-157899: technical: rename HAVE_FREADLINK C macro to _Py_HAVE…

8196b59

…_FREADLINK.

OOTS force-pushed the feature/gh-157899-fds-in-os-readlink branch from d7fa0ad to 8196b59 Compare September 23, 2026 10:18

encukou reviewed Sep 24, 2026

View reviewed changes

OOTS added 2 commits September 27, 2026 11:53

pythongh-157889: fix breaking tests for emscripten.

7b566b7

Emscripten apparrently defines O_PATH and readlinkat - even though it doesn't actually support them. This broke auto-detection of whether os.readlink supports FDs - which broke the tests.

Uh oh!

Conversation

OOTS commented Sep 21, 2026 • edited by bedevere-app Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

python-cla-bot Bot commented Sep 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

read-the-docs-community Bot commented Sep 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Documentation build overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OOTS commented Sep 21, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OOTS commented Sep 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

OOTS commented Sep 21, 2026 •

edited by bedevere-app Bot

Loading

python-cla-bot Bot commented Sep 21, 2026 •

edited

Loading

read-the-docs-community Bot commented Sep 21, 2026 •

edited

Loading