From 3008727467bea32001e3b127d7e24e692aa63052 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Sat, 7 Sep 2024 12:15:40 +0300 Subject: [PATCH 1/4] gh-123797: Check for runtime availability of `ptsname_r` on macos --- Lib/test/test_posix.py | 7 +++++++ ...-09-07-12-14-54.gh-issue-123797.yFDeug.rst | 1 + Modules/posixmodule.c | 20 ++++++++++++++++++- 3 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/macOS/2024-09-07-12-14-54.gh-issue-123797.yFDeug.rst diff --git a/Lib/test/test_posix.py b/Lib/test/test_posix.py index 908354cb8574d1..5d2decffd720f0 100644 --- a/Lib/test/test_posix.py +++ b/Lib/test/test_posix.py @@ -2140,6 +2140,13 @@ def test_stat(self): with self.assertRaisesRegex(NotImplementedError, "dir_fd unavailable"): os.stat("file", dir_fd=0) + def test_ptsname_r(self): + self._verify_available("HAVE_PTSNAME_R") + if self.mac_ver >= (10, 13, 4): + self.assertIn("HAVE_PTSNAME_R", posix._have_functions) + else: + self.assertNotIn("HAVE_PTSNAME_R", posix._have_functions) + def test_access(self): self._verify_available("HAVE_FACCESSAT") if self.mac_ver >= (10, 10): diff --git a/Misc/NEWS.d/next/macOS/2024-09-07-12-14-54.gh-issue-123797.yFDeug.rst b/Misc/NEWS.d/next/macOS/2024-09-07-12-14-54.gh-issue-123797.yFDeug.rst new file mode 100644 index 00000000000000..f126bd0d39bf59 --- /dev/null +++ b/Misc/NEWS.d/next/macOS/2024-09-07-12-14-54.gh-issue-123797.yFDeug.rst @@ -0,0 +1 @@ +Check for runtime availability of ``ptsname_r`` function on macos. diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index f24ab81cbcb77b..69f11d780f339b 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -125,6 +125,7 @@ # define HAVE_PWRITEV_RUNTIME __builtin_available(macOS 11.0, iOS 14.0, tvOS 14.0, watchOS 7.0, *) # define HAVE_MKFIFOAT_RUNTIME __builtin_available(macOS 13.0, iOS 16.0, tvOS 16.0, watchOS 9.0, *) # define HAVE_MKNODAT_RUNTIME __builtin_available(macOS 13.0, iOS 16.0, tvOS 16.0, watchOS 9.0, *) +# define HAVE_PTSNAME_R_RUNTIME __builtin_available(macOS 10.13.4, iOS 11.3, tvOS 11.3, watchOS 4.3, *) # define HAVE_POSIX_SPAWN_SETSID_RUNTIME __builtin_available(macOS 10.15, *) @@ -206,6 +207,10 @@ # define HAVE_MKNODAT_RUNTIME (mknodat != NULL) # endif +# ifdef HAVE_PTSNAME_R +# define HAVE_PTSNAME_R_RUNTIME (ptsname_r != NULL) +# endif + #endif #ifdef HAVE_FUTIMESAT @@ -231,6 +236,7 @@ # define HAVE_PWRITEV_RUNTIME 1 # define HAVE_MKFIFOAT_RUNTIME 1 # define HAVE_MKNODAT_RUNTIME 1 +# define HAVE_PTSNAME_R_RUNTIME 1 #endif @@ -8656,7 +8662,12 @@ os_ptsname_impl(PyObject *module, int fd) int ret; char name[MAXPATHLEN+1]; - ret = ptsname_r(fd, name, sizeof(name)); + if (HAVE_PTSNAME_R_RUNTIME) { + ret = ptsname_r(fd, name, sizeof(name)); + } + else { + ret = -1; + } if (ret != 0) { errno = ret; return posix_error(); @@ -17751,6 +17762,9 @@ PROBE(probe_futimens, HAVE_FUTIMENS_RUNTIME) PROBE(probe_utimensat, HAVE_UTIMENSAT_RUNTIME) #endif +#ifdef HAVE_PTSNAME_R +PROBE(probe_ptsname_r, HAVE_PTSNAME_R_RUNTIME) +#endif @@ -17891,6 +17905,10 @@ static const struct have_function { { "HAVE_UTIMENSAT", probe_utimensat }, #endif +#ifdef HAVE_PTSNAME_R + { "HAVE_PTSNAME_R", probe_ptsname_r }, +#endif + #ifdef MS_WINDOWS { "MS_WINDOWS", NULL }, #endif From 01f0cd0e082a3f0cbd6200be4a52821dfc4fdbe8 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Mon, 9 Sep 2024 20:53:10 +0300 Subject: [PATCH 2/4] Fallback to `ptsname` when it is available --- Modules/posixmodule.c | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 69f11d780f339b..9b19017f69a556 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -8641,6 +8641,18 @@ os_unlockpt_impl(PyObject *module, int fd) #endif /* HAVE_UNLOCKPT */ #if defined(HAVE_PTSNAME) || defined(HAVE_PTSNAME_R) +static PyObject * +from_ptsname(int fd) +{ + char *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(); + } + return PyUnicode_DecodeFSDefault(name); +} + /*[clinic input] os.ptsname @@ -8666,23 +8678,24 @@ os_ptsname_impl(PyObject *module, int fd) ret = ptsname_r(fd, name, sizeof(name)); } else { +#if defined(HAVE_PTSNAME) + // fallback to `ptsname` if `ptsname_r` is not available in runtime. + return from_ptsname(fd); +#else + // unknown error: + // both ptsname_r and ptsname are not available for some reason. ret = -1; +#endif /* HAVE_PTSNAME */ } 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); +#else + return from_ptsname(fd); +#endif /* HAVE_PTSNAME_R */ } #endif /* defined(HAVE_PTSNAME) || defined(HAVE_PTSNAME_R) */ From 58d28d17da4c2cb253f4dbdf089bfdf39cd48698 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Mon, 9 Sep 2024 22:17:32 +0300 Subject: [PATCH 3/4] Address review --- Modules/posixmodule.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 9b19017f69a556..f93a554eec612e 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -8678,14 +8678,8 @@ os_ptsname_impl(PyObject *module, int fd) ret = ptsname_r(fd, name, sizeof(name)); } else { -#if defined(HAVE_PTSNAME) // fallback to `ptsname` if `ptsname_r` is not available in runtime. return from_ptsname(fd); -#else - // unknown error: - // both ptsname_r and ptsname are not available for some reason. - ret = -1; -#endif /* HAVE_PTSNAME */ } if (ret != 0) { errno = ret; From 35fdc2852da870c3772a4ae5c942f06f801c5822 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Tue, 10 Sep 2024 19:04:56 +0300 Subject: [PATCH 4/4] Address review --- Modules/posixmodule.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index f93a554eec612e..7b91c47297b4de 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -8642,11 +8642,12 @@ os_unlockpt_impl(PyObject *module, int fd) #if defined(HAVE_PTSNAME) || defined(HAVE_PTSNAME_R) static PyObject * -from_ptsname(int fd) +py_ptsname(int fd) { + // POSIX manpage: Upon failure, ptsname() shall return a null pointer + // and may set errno. Always initialize errno to avoid undefined behavior. + errno = 0; char *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(); } @@ -8678,8 +8679,8 @@ os_ptsname_impl(PyObject *module, int fd) ret = ptsname_r(fd, name, sizeof(name)); } else { - // fallback to `ptsname` if `ptsname_r` is not available in runtime. - return from_ptsname(fd); + // fallback to ptsname() if ptsname_r() is not available in runtime. + return py_ptsname(fd); } if (ret != 0) { errno = ret; @@ -8688,7 +8689,7 @@ os_ptsname_impl(PyObject *module, int fd) return PyUnicode_DecodeFSDefault(name); #else - return from_ptsname(fd); + return py_ptsname(fd); #endif /* HAVE_PTSNAME_R */ } #endif /* defined(HAVE_PTSNAME) || defined(HAVE_PTSNAME_R) */