Update os.ptsname() so that it uses the C function ptsname_r() if it …

…is available. Make os.grantpt() use saved errno upon failure. Update documentation related to the POSIX pty functions. Add a test for the POSIX pty functions. Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com>
python · encukou · Jan 29, 2024 · Mar 15, 2023 · Mar 15, 2023 · Mar 15, 2023
commit 3effe4b5c6ab117612ffd6f6e31e446cc12ed258
diff --git a/Doc/library/os.rst b/Doc/library/os.rst
@@ -1127,6 +1127,7 @@ as internal buffering of data.
 
    Grant access to the slave pseudo-terminal device associated with the
    master pseudo-terminal device to which the file descriptor *fd* refers.
+   The file descriptor *fd* is not closed upon failure.
 
    Calls the C standard library function :c:func:`grantpt`.
 
@@ -1450,6 +1451,10 @@ or `the MSDN <https://msdn.microsoft.com/en-us/library/z0kc8e3z.aspx>`_ on Windo
    argument is used to set file status flags and file access modes as
    specified in the manual page of :c:func:`posix_openpt` of your system.
 
+   The returned file descriptor is :ref:`non-inheritable <fd_inheritance>`.
+   If the value :data:`O_CLOEXEC` is available on the system, it is added to
+   *oflag*.
+
    .. availability:: Unix, not Emscripten, not WASI.
 
    .. versionadded:: 3.13
@@ -1516,9 +1521,11 @@ or `the MSDN <https://msdn.microsoft.com/en-us/library/z0kc8e3z.aspx>`_ on Windo
 
    Return the name of the slave pseudo-terminal device associated with the
    master pseudo-terminal device to which the file descriptor *fd* refers.
+   The file descriptor *fd* is not closed upon failure.
 
-   Calls the C standard library function :c:func:`ptsname`, which is not
-   guaranteed to be thread-safe.
+   Calls the reentrant C standard library function :c:func:`ptsname_r` if
+   it is available; otherwise, the C standard library function
+   :c:func:`ptsname`, which is not guaranteed to be thread-safe, is called.
 
    .. availability:: Unix, not Emscripten, not WASI.
 
@@ -1781,6 +1788,7 @@ or `the MSDN <https://msdn.microsoft.com/en-us/library/z0kc8e3z.aspx>`_ on Windo
 
    Unlock the slave pseudo-terminal device associated with the master
    pseudo-terminal device to which the file descriptor *fd* refers.
+   The file descriptor *fd* is not closed upon failure.
 
    Calls the C standard library function :c:func:`unlockpt`.
 

diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py
@@ -4492,23 +4492,32 @@ def test_openpty(self):
         self.assertEqual(os.get_inheritable(second_fd), False)
 
     @unittest.skipUnless(hasattr(os, 'ptsname'), "need os.ptsname()")
-    def test_ptsname(self):
-        main_fd, second_fd = self.open_pty()
-        self.assertEqual(os.ptsname(main_fd), os.ttyname(second_fd))
-
-    @unittest.skipUnless(hasattr(os, 'ptsname'), "need os.ptsname()")
-    @unittest.skipUnless(hasattr(os, 'grantpt'), "need os.grantpt()")
-    @unittest.skipUnless(hasattr(os, 'unlockpt'), "need os.unlockpt()")
+    @unittest.skipUnless(hasattr(os, 'O_RDWR'), "need os.O_RDWR")
+    @unittest.skipUnless(hasattr(os, 'O_NOCTTY'), "need os.O_NOCTTY")
     def test_open_via_ptsname(self):
         main_fd, second_fd = self.open_pty()
-        os.grantpt(main_fd)
-        os.unlockpt(main_fd)
         second_path = os.ptsname(main_fd)
         reopened_second_fd = os.open(second_path, os.O_RDWR|os.O_NOCTTY)
         self.addCleanup(os.close, reopened_second_fd)
         os.write(reopened_second_fd, b'foo')
         self.assertEqual(os.read(main_fd, 3), b'foo')
 
+    @unittest.skipUnless(hasattr(os, 'posix_openpt'), "need os.posix_openpt()")
+    @unittest.skipUnless(hasattr(os, 'grantpt'), "need os.grantpt()")
+    @unittest.skipUnless(hasattr(os, 'unlockpt'), "need os.unlockpt()")
+    @unittest.skipUnless(hasattr(os, 'ptsname'), "need os.ptsname()")
+    @unittest.skipUnless(hasattr(os, 'O_RDWR'), "need os.O_RDWR")
+    @unittest.skipUnless(hasattr(os, 'O_NOCTTY'), "need os.O_NOCTTY")
+    def test_posix_pty_functions(self):
+        mother_fd = os.posix_openpt(os.O_RDWR|os.O_NOCTTY)
+        self.addCleanup(os.close, mother_fd)
+        os.grantpt(mother_fd)
+        os.unlockpt(mother_fd)
+        son_path = os.ptsname(mother_fd)
+        son_fd = os.open(son_path, os.O_RDWR|os.O_NOCTTY)
+        self.addCleanup(os.close, son_fd)
+        self.assertEqual(os.ptsname(mother_fd), os.ttyname(son_fd))
+
 
 class PathTConverterTests(unittest.TestCase):
     # tuples of (function name, allows fd arguments, additional arguments to

diff --git a/Modules/clinic/posixmodule.c.h b/Modules/clinic/posixmodule.c.h
diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c
@@ -8412,14 +8412,21 @@ os_grantpt_impl(PyObject *module, int fd)
 /*[clinic end generated code: output=dfd580015cf548ab input=0668e3b96760e849]*/
 {
     int ret;
+    int saved_errno;
     PyOS_sighandler_t sig_saved;
 
     sig_saved = PyOS_setsig(SIGCHLD, SIG_DFL);
+
     ret = grantpt(fd);
+    if (ret == -1)
+        saved_errno = errno;
+
     PyOS_setsig(SIGCHLD, sig_saved);
 
-    if (ret == -1)
+    if (ret == -1) {
+        errno = saved_errno;
         return posix_error();
+    }
 
     Py_RETURN_NONE;
 }
@@ -8449,7 +8456,7 @@ os_unlockpt_impl(PyObject *module, int fd)
 }
 #endif /* HAVE_UNLOCKPT */
 
-#ifdef HAVE_PTSNAME
+#if defined(HAVE_PTSNAME) || defined(HAVE_PTSNAME_R)
 /*[clinic input]
 os.ptsname
 
@@ -8459,22 +8466,36 @@ os.ptsname
 
 Return the name of the slave pseudo-terminal device.
 
-Performs a ptsname() C function call.
+If the ptsname_r() C function is available, it is called;
+otherwise, performs a ptsname() C function call.
 [clinic start generated code]*/
 
 static PyObject *
 os_ptsname_impl(PyObject *module, int fd)
-/*[clinic end generated code: output=ef300fadc5675872 input=a00d870c51570c2a]*/
+/*[clinic end generated code: output=ef300fadc5675872 input=1369ccc0546f3130]*/
 {
+#ifdef HAVE_PTSNAME_R
+    int ret;
+    char name[MAXPATHLEN+1];
+
+    ret = ptsname_r(fd, name, sizeof(name));
+    if (ret != 0) {
+        errno = ret;
+        return posix_error();
+    }
+#else
     char *name;
 
     name = ptsname(fd);
+    /* POSIX manpage: Upon failure, ptsname() shall return a null pointer and may set errno.
+       *MAY* set errno? Hmm... */
     if (name == NULL)
         return posix_error();
+#endif /* HAVE_PTSNAME_R */
 
     return PyUnicode_DecodeFSDefault(name);
 }
-#endif /* HAVE_PTSNAME */
+#endif /* defined(HAVE_PTSNAME) || defined(HAVE_PTSNAME_R) */
 
 /* AIX uses /dev/ptc but is otherwise the same as /dev/ptmx */
 #if defined(HAVE_DEV_PTC) && !defined(HAVE_DEV_PTMX)

diff --git a/aclocal.m4 b/aclocal.m4