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.

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

Documentation build overview

📚 cpython-previews | 🛠️ Build #35062675 | 📁 Comparing c8d85a6 against main (a6ad448)

🔍 Preview build

53 files changed · + 1 added · ± 52 modified

+ Added

library/dialog.html

± Modified

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

_support_readlink_with_fd should now be equivalent to hasattr(os, 'readlink') and (os.readlink in os.supports_fd). Or more fancy, (getattr(os, 'readlink', None) in os.supports_fd)
The in expression can be used directly, as in the test above; no need for a _support_readlink_with_fd wariable.

I'm updating this, but I'd prefer to keep the _support_readlink_with_fd variable (just for avoiding code/logic duplication).

As a side effect, I'm now removing the self.assertEqual(os.readlink in os.supports_fd, self._support_readlink_with_fd) test that you asked for, since this test would be tautological. (It currently sits behind a @unittest.skipUnless(hasattr(os, "readlink")) guard.)

Okay, rather than removing the test completely I added something closer to what you originally asked for:

@unittest.skipUnless(sys.platform in ["linux", "android", "darwin", "ios"], "feature not supported on this platform") def test_readlink_is_marked_as_supports_fd(self): self.assertIn(os.readlink, os.supports_fd)

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

vstinner

macOS should be written "macOS", not "MacOS".

After changing return type of os.readlink when called with an fd from bytes to string, missed one place in the docs that needed updating.

* Simplified/unified error handling in case reading fds referring to symlinks is not supported. * Docs: centralized the information on the return type of os.readlink in a single paragraph. * Docs/comments: rewrite from "MacOS" to "macOS" (lower-case).

vstinner

Overall, the change LGTM. I just have another minor comment.

@encukou: I let you double check the change :-)

I was curious and replaced readlinkat(path->fd, "", buffer, MAXPATHLEN) with readlinkat(path->fd, "", buffer, MAXPATHLEN). With this change, the function fails with OSError: [Errno 14] Bad address: 6. So no, it's not possible to pass NULL: passing an empty string is the right argument.

vstinner · 2026-09-29T14:31:23Z

+    @unittest.skipUnless(hasattr(os, "supports_fd") and hasattr(os, "readlink"),
+                         "feature not supported on this platform")


According to other tests, hasattr(os, "supports_fd") test is not needed, the attribute can always be used.

Suggested change

@unittest.skipUnless(hasattr(os, "supports_fd") and hasattr(os, "readlink"),

"feature not supported on this platform")

@unittest.skipUnless(hasattr(os, "readlink"), "need os.readlink")

Sorry, I missed this comment. I just pushed an update: 6c8f5f1

Originally I wanted to be careful (because the supports_fd set is only defined within an if _exists("_have_functions") guard), but it appears that the _have_functions list is defined unconditionally (at least in the posixmodule.c).

encukou · 2026-09-30T07:04:47Z

@encukou: I let you double check the change :-)

I will, after the 3.15.0 release. Please don't merge yet.

…s-in-os-readlink and resolved merge conflicts.

OOTS · 2026-10-09T14:32:35Z

Resolved a merge conflict with commit cfd6cf7 on a "clinic end generated code" line: cfd6cf71547e#diff-f4ec448d9bc73d373452e97931de916ca83058ae93bce4a5853b8ac18c9d2cfa

encukou

Thank you! This is looking great, I have a few cleanups to suggest:

encukou · 2026-10-02T14:37:35Z

+    _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"]
+        )
+    )


_support_readlink_with_fd should now be equivalent to hasattr(os, 'readlink') and (os.readlink in os.supports_fd). Or more fancy, (getattr(os, 'readlink', None) in os.supports_fd)
The in expression can be used directly, as in the test above; no need for a _support_readlink_with_fd wariable.

encukou · 2026-10-02T14:42:14Z

+    def test_readlink_with_fd_throws_not_implemented_error(self):
+        # on unsupported platforms, we may not even be able to get a
+        # file descriptor for a symlink, so use a fd for an ordinary file
+        os_helper.create_empty_file(os_helper.TESTFN)
+        self.addCleanup(os_helper.unlink, os_helper.TESTFN)
+        fd = os.open(os_helper.TESTFN, os.O_RDONLY)
+        self.addCleanup(os.close, fd)
+        with self.assertRaises(NotImplementedError):
+            os.readlink(fd)


Could you also test that _open_symlink_as_fd raises?
Such a test might be too eager, if it is we* can always remove it. But, if a platform has either O_SYMLINK or O_NOFOLLOW | O_PATH, it would be weird if it didn't also support readlink with fd.

* (or someone porting to a new platform, after the tests warn them about the issue)

Hmm, from what I can see iOS has O_SYMLINK in SDK version 9.3: https://github.com/xybp888/iOS-SDKs/blob/ad607cb07fe4ad1c9b91cf970bd228c2a9253207/iPhoneOS9.3.sdk/usr/include/sys/fcntl.h#L154

I also found that apparently iOS does have freadlink starting in version 16.0: https://github.com/apple-oss-distributions/xnu/blob/f6217f891ac0bb64f3d375211650a4c1ff8ca1ea/bsd/sys/unistd.h#L192 . So I'll update this MR to support freadlink on iOS >= 16.0, but the test you suggest would break on iOS < 16.0.

encukou · 2026-10-09T14:51:58Z

+On Linux, Android and macOS, path may be a file descriptor referring to
+a symlink. If it is, dir_fd must be None, and the return value will be a
+string object. (File descriptors for symlinks can be obtained with
+
+    os.open(..., os.O_RDONLY | os.O_PATH | os.O_NOFOLLOW)
+
+on Linux and Android, and:
+
+    os.open(..., os.O_RDONLY | os.O_SYMLINK)
+
+on macOS.)


"the return value will be a string object" was already mentioned.

The usage notes are better left to the main docs.

Suggested change

On Linux, Android and macOS, path may be a file descriptor referring to

a symlink. If it is, dir_fd must be None, and the return value will be a

string object. (File descriptors for symlinks can be obtained with

os.open(..., os.O_RDONLY | os.O_PATH | os.O_NOFOLLOW)

on Linux and Android, and:

os.open(..., os.O_RDONLY | os.O_SYMLINK)

on macOS.)

On Linux, Android and macOS, path may be a file descriptor referring to

a symlink. If it is, dir_fd must be None.

done. (Also added iOS to the docs here, in the os.rst, whatsnew/3.16.rst and the NEWS.d.)

encukou · 2026-10-09T14:57:49Z

+    if (path->is_fd || PyUnicode_Check(path->object))
        return PyUnicode_DecodeFSDefaultAndSize(buffer, length);
    else
        return PyBytes_FromStringAndSize(buffer, length);


Update to PEP-7 style when touching the code:

Suggested change

if (path->is_fd || PyUnicode_Check(path->object))

return PyUnicode_DecodeFSDefaultAndSize(buffer, length);

else

return PyBytes_FromStringAndSize(buffer, length);

if (path->is_fd || PyUnicode_Check(path->object)) {

return PyUnicode_DecodeFSDefaultAndSize(buffer, length);

}

else {

return PyBytes_FromStringAndSize(buffer, length);

}

…ink with a FD referring to a symlink. Also some code/test cleanup.

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

Comment thread Modules/posixmodule.c

OOTS commented Sep 21, 2026

View reviewed changes

Comment thread Lib/test/test_os/test_posix.py

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.

OOTS requested a review from encukou September 28, 2026 07:45

bedevere-app Bot added the type-feature A feature request or enhancement label Sep 28, 2026

vstinner reviewed Sep 28, 2026

View reviewed changes

OOTS added 2 commits September 29, 2026 14:26

pythongh-157899: fix docs.

253c3b4

After changing return type of os.readlink when called with an fd from bytes to string, missed one place in the docs that needed updating.

OOTS force-pushed the feature/gh-157899-fds-in-os-readlink branch from 6094134 to 20fa1e2 Compare September 29, 2026 14:20

vstinner approved these changes Sep 29, 2026

View reviewed changes

bedevere-app Bot added awaiting merge and removed awaiting review labels Sep 29, 2026

OOTS added 2 commits October 9, 2026 12:12

pythongh-157899: removed unnecessary guard for test.

6c8f5f1

pythongh-157899: merged upstream/main into feature/pythongh-157899-fd…

9bf9236

…s-in-os-readlink and resolved merge conflicts.

encukou reviewed Oct 9, 2026

View reviewed changes

pythongh-157899: added iOS as supported platform for calling os.readl…

c8d85a6

…ink with a FD referring to a symlink. Also some code/test cleanup.

		@unittest.skipUnless(hasattr(os, "supports_fd") and hasattr(os, "readlink"),
		"feature not supported on this platform")

	@unittest.skipUnless(hasattr(os, "supports_fd") and hasattr(os, "readlink"),
	"feature not supported on this platform")
	@unittest.skipUnless(hasattr(os, "readlink"), "need os.readlink")

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!

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!

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

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!

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!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

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