From 4d5ae6663203ff4ab646933d11a3aa57b772f3ed Mon Sep 17 00:00:00 2001 From: sobolevn Date: Wed, 4 Oct 2023 18:57:02 +0300 Subject: [PATCH 1/9] gh-110365: Fix error overwrite in `termios.tcsetattr` --- ...-10-04-18-56-29.gh-issue-110365.LCxiau.rst | 2 ++ Modules/termios.c | 30 ++++++++++++++----- 2 files changed, 24 insertions(+), 8 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2023-10-04-18-56-29.gh-issue-110365.LCxiau.rst diff --git a/Misc/NEWS.d/next/Library/2023-10-04-18-56-29.gh-issue-110365.LCxiau.rst b/Misc/NEWS.d/next/Library/2023-10-04-18-56-29.gh-issue-110365.LCxiau.rst new file mode 100644 index 00000000000000..456818bb625518 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-10-04-18-56-29.gh-issue-110365.LCxiau.rst @@ -0,0 +1,2 @@ +Fix :func:`termios.tcsetattr` bug that was overwritting existing errors +during parsing integets from ``term`` list. diff --git a/Modules/termios.c b/Modules/termios.c index 6f07c93bcdb6b5..b55395f29c71fc 100644 --- a/Modules/termios.c +++ b/Modules/termios.c @@ -212,17 +212,31 @@ termios_tcsetattr_impl(PyObject *module, int fd, int when, PyObject *term) return PyErr_SetFromErrno(state->TermiosError); } - mode.c_iflag = (tcflag_t) PyLong_AsLong(PyList_GetItem(term, 0)); - mode.c_oflag = (tcflag_t) PyLong_AsLong(PyList_GetItem(term, 1)); - mode.c_cflag = (tcflag_t) PyLong_AsLong(PyList_GetItem(term, 2)); - mode.c_lflag = (tcflag_t) PyLong_AsLong(PyList_GetItem(term, 3)); - speed_t ispeed = (speed_t) PyLong_AsLong(PyList_GetItem(term, 4)); - speed_t ospeed = (speed_t) PyLong_AsLong(PyList_GetItem(term, 5)); - PyObject *cc = PyList_GetItem(term, 6); + long item; +#define SAFE_LONG_ITEM(obj) \ + item = PyLong_AsLong(obj); \ + if (PyErr_Occurred()) { \ + return NULL; \ + } + + SAFE_LONG_ITEM(PyList_GET_ITEM(term, 0)); + mode.c_iflag = (tcflag_t) item; + SAFE_LONG_ITEM(PyList_GET_ITEM(term, 1)); + mode.c_oflag = (tcflag_t) item; + SAFE_LONG_ITEM(PyList_GET_ITEM(term, 2)); + mode.c_cflag = (tcflag_t) item; + SAFE_LONG_ITEM(PyList_GET_ITEM(term, 3)); + mode.c_lflag = (tcflag_t) item; + SAFE_LONG_ITEM(PyList_GET_ITEM(term, 4)); + speed_t ispeed = (speed_t) item; + SAFE_LONG_ITEM(PyList_GET_ITEM(term, 5)); + speed_t ospeed = (speed_t) item; +#undef SAFE_LONG_ITEM + + PyObject *cc = PyList_GET_ITEM(term, 6); if (PyErr_Occurred()) { return NULL; } - if (!PyList_Check(cc) || PyList_Size(cc) != NCCS) { PyErr_Format(PyExc_TypeError, "tcsetattr: attributes[6] must be %d element list", From d1d9d2a63c3901c3631ed8e51f919dca4e8031b0 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Wed, 4 Oct 2023 22:18:30 +0300 Subject: [PATCH 2/9] Address review --- Modules/termios.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Modules/termios.c b/Modules/termios.c index b55395f29c71fc..3428d3a42ff82b 100644 --- a/Modules/termios.c +++ b/Modules/termios.c @@ -215,7 +215,7 @@ termios_tcsetattr_impl(PyObject *module, int fd, int when, PyObject *term) long item; #define SAFE_LONG_ITEM(obj) \ item = PyLong_AsLong(obj); \ - if (PyErr_Occurred()) { \ + if (item == -1 && PyErr_Occurred()) { \ return NULL; \ } @@ -234,9 +234,6 @@ termios_tcsetattr_impl(PyObject *module, int fd, int when, PyObject *term) #undef SAFE_LONG_ITEM PyObject *cc = PyList_GET_ITEM(term, 6); - if (PyErr_Occurred()) { - return NULL; - } if (!PyList_Check(cc) || PyList_Size(cc) != NCCS) { PyErr_Format(PyExc_TypeError, "tcsetattr: attributes[6] must be %d element list", From 693f95107ef92160feda8369fdac7ee0ef641075 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Wed, 4 Oct 2023 22:45:07 +0300 Subject: [PATCH 3/9] One more problem --- Modules/termios.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/Modules/termios.c b/Modules/termios.c index 3428d3a42ff82b..eee986ed88450e 100644 --- a/Modules/termios.c +++ b/Modules/termios.c @@ -248,9 +248,13 @@ termios_tcsetattr_impl(PyObject *module, int fd, int when, PyObject *term) if (PyBytes_Check(v) && PyBytes_Size(v) == 1) mode.c_cc[i] = (cc_t) * PyBytes_AsString(v); - else if (PyLong_Check(v)) - mode.c_cc[i] = (cc_t) PyLong_AsLong(v); - else { + else if (PyLong_Check(v)) { + item = PyLong_AsLong(v); + if (item == -1 && PyErr_Occurred()) { + return NULL; + } + mode.c_cc[i] = (cc_t) item; + } else { PyErr_SetString(PyExc_TypeError, "tcsetattr: elements of attributes must be characters or integers"); return NULL; From 33d86b2309f9d68ce114123f626dbc649fcba080 Mon Sep 17 00:00:00 2001 From: Nikita Sobolev Date: Thu, 5 Oct 2023 11:07:26 +0300 Subject: [PATCH 4/9] Apply suggestions from code review Co-authored-by: Erlend E. Aasland --- ...-10-04-18-56-29.gh-issue-110365.LCxiau.rst | 2 +- Modules/termios.c | 38 +++++++++---------- 2 files changed, 18 insertions(+), 22 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2023-10-04-18-56-29.gh-issue-110365.LCxiau.rst b/Misc/NEWS.d/next/Library/2023-10-04-18-56-29.gh-issue-110365.LCxiau.rst index 456818bb625518..a1ac39b60296a3 100644 --- a/Misc/NEWS.d/next/Library/2023-10-04-18-56-29.gh-issue-110365.LCxiau.rst +++ b/Misc/NEWS.d/next/Library/2023-10-04-18-56-29.gh-issue-110365.LCxiau.rst @@ -1,2 +1,2 @@ Fix :func:`termios.tcsetattr` bug that was overwritting existing errors -during parsing integets from ``term`` list. +during parsing integers from ``term`` list. diff --git a/Modules/termios.c b/Modules/termios.c index eee986ed88450e..1e24afdca66684 100644 --- a/Modules/termios.c +++ b/Modules/termios.c @@ -212,26 +212,21 @@ termios_tcsetattr_impl(PyObject *module, int fd, int when, PyObject *term) return PyErr_SetFromErrno(state->TermiosError); } - long item; -#define SAFE_LONG_ITEM(obj) \ - item = PyLong_AsLong(obj); \ - if (item == -1 && PyErr_Occurred()) { \ - return NULL; \ - } - - SAFE_LONG_ITEM(PyList_GET_ITEM(term, 0)); - mode.c_iflag = (tcflag_t) item; - SAFE_LONG_ITEM(PyList_GET_ITEM(term, 1)); - mode.c_oflag = (tcflag_t) item; - SAFE_LONG_ITEM(PyList_GET_ITEM(term, 2)); - mode.c_cflag = (tcflag_t) item; - SAFE_LONG_ITEM(PyList_GET_ITEM(term, 3)); - mode.c_lflag = (tcflag_t) item; - SAFE_LONG_ITEM(PyList_GET_ITEM(term, 4)); - speed_t ispeed = (speed_t) item; - SAFE_LONG_ITEM(PyList_GET_ITEM(term, 5)); - speed_t ospeed = (speed_t) item; -#undef SAFE_LONG_ITEM +#define SET_FROM_LIST(TYPE, VAR, LIST, N) do { \ + PyObject *item = PyList_GET_ITEM(LIST, N); \ + long num = PyLong_AsLong(item); \ + if (num == -1 && PyErr_Occurred()) { \ + return NULL; \ + } \ + VAR = (TYPE)num; \ +} while (0) + SET_FROM_LIST(tcflag_t, mode.c_iflag, term, 0); + SET_FROM_LIST(tcflag_t, mode.c_oflag, term, 1); + SET_FROM_LIST(tcflag_t, mode.c_cflag, term, 2); + SET_FROM_LIST(tcflag_t, mode.c_lflag, term, 3); + SET_FROM_LIST(speed_t, ispeed, term, 4); + SET_FROM_LIST(speed_t, ospeed, term, 5); +#undef SET_FROM_LIST PyObject *cc = PyList_GET_ITEM(term, 6); if (!PyList_Check(cc) || PyList_Size(cc) != NCCS) { @@ -254,7 +249,8 @@ termios_tcsetattr_impl(PyObject *module, int fd, int when, PyObject *term) return NULL; } mode.c_cc[i] = (cc_t) item; - } else { + } + else { PyErr_SetString(PyExc_TypeError, "tcsetattr: elements of attributes must be characters or integers"); return NULL; From 9f7c57d98a7b99d76c555cfd91370d69ea841145 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Thu, 5 Oct 2023 11:18:38 +0300 Subject: [PATCH 5/9] Fix CI --- Modules/termios.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/Modules/termios.c b/Modules/termios.c index 1e24afdca66684..52ef170090ac66 100644 --- a/Modules/termios.c +++ b/Modules/termios.c @@ -212,14 +212,16 @@ termios_tcsetattr_impl(PyObject *module, int fd, int when, PyObject *term) return PyErr_SetFromErrno(state->TermiosError); } + speed_t ispeed, ospeed; #define SET_FROM_LIST(TYPE, VAR, LIST, N) do { \ PyObject *item = PyList_GET_ITEM(LIST, N); \ long num = PyLong_AsLong(item); \ if (num == -1 && PyErr_Occurred()) { \ return NULL; \ } \ - VAR = (TYPE)num; \ + VAR = (TYPE) num; \ } while (0) + SET_FROM_LIST(tcflag_t, mode.c_iflag, term, 0); SET_FROM_LIST(tcflag_t, mode.c_oflag, term, 1); SET_FROM_LIST(tcflag_t, mode.c_cflag, term, 2); @@ -238,17 +240,18 @@ termios_tcsetattr_impl(PyObject *module, int fd, int when, PyObject *term) int i; PyObject *v; + long l; for (i = 0; i < NCCS; i++) { v = PyList_GetItem(cc, i); if (PyBytes_Check(v) && PyBytes_Size(v) == 1) mode.c_cc[i] = (cc_t) * PyBytes_AsString(v); else if (PyLong_Check(v)) { - item = PyLong_AsLong(v); - if (item == -1 && PyErr_Occurred()) { + l = PyLong_AsLong(v); + if (l == -1 && PyErr_Occurred()) { return NULL; } - mode.c_cc[i] = (cc_t) item; + mode.c_cc[i] = (cc_t) l; } else { PyErr_SetString(PyExc_TypeError, From b04ba927ed4fcd8d5a92bd256373f3ad8f2bbf84 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Thu, 5 Oct 2023 11:21:05 +0300 Subject: [PATCH 6/9] Better var name --- Modules/termios.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Modules/termios.c b/Modules/termios.c index 52ef170090ac66..7a7ec8f50388ae 100644 --- a/Modules/termios.c +++ b/Modules/termios.c @@ -240,15 +240,15 @@ termios_tcsetattr_impl(PyObject *module, int fd, int when, PyObject *term) int i; PyObject *v; - long l; + long lng; for (i = 0; i < NCCS; i++) { v = PyList_GetItem(cc, i); if (PyBytes_Check(v) && PyBytes_Size(v) == 1) mode.c_cc[i] = (cc_t) * PyBytes_AsString(v); else if (PyLong_Check(v)) { - l = PyLong_AsLong(v); - if (l == -1 && PyErr_Occurred()) { + lng = PyLong_AsLong(v); + if (lng == -1 && PyErr_Occurred()) { return NULL; } mode.c_cc[i] = (cc_t) l; From 100b15f556cd70aa1a037b0fb596527d5353d263 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Thu, 5 Oct 2023 11:24:08 +0300 Subject: [PATCH 7/9] Better var name --- Modules/termios.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/termios.c b/Modules/termios.c index 7a7ec8f50388ae..7894ebe80bfd61 100644 --- a/Modules/termios.c +++ b/Modules/termios.c @@ -251,7 +251,7 @@ termios_tcsetattr_impl(PyObject *module, int fd, int when, PyObject *term) if (lng == -1 && PyErr_Occurred()) { return NULL; } - mode.c_cc[i] = (cc_t) l; + mode.c_cc[i] = (cc_t) lng; } else { PyErr_SetString(PyExc_TypeError, From 2a1a340e71a44af6411807e24045f6e4e5cd6035 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Thu, 5 Oct 2023 11:24:41 +0300 Subject: [PATCH 8/9] Better var name --- Modules/termios.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Modules/termios.c b/Modules/termios.c index 7894ebe80bfd61..87af25a53bbc89 100644 --- a/Modules/termios.c +++ b/Modules/termios.c @@ -240,14 +240,13 @@ termios_tcsetattr_impl(PyObject *module, int fd, int when, PyObject *term) int i; PyObject *v; - long lng; for (i = 0; i < NCCS; i++) { v = PyList_GetItem(cc, i); if (PyBytes_Check(v) && PyBytes_Size(v) == 1) mode.c_cc[i] = (cc_t) * PyBytes_AsString(v); else if (PyLong_Check(v)) { - lng = PyLong_AsLong(v); + long lng = PyLong_AsLong(v); if (lng == -1 && PyErr_Occurred()) { return NULL; } From 34e437bb73d698ccf0e47875e05ae3c24628d42f Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Thu, 5 Oct 2023 10:29:59 +0200 Subject: [PATCH 9/9] Nits --- Modules/termios.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Modules/termios.c b/Modules/termios.c index 87af25a53bbc89..9fc2673ce0e788 100644 --- a/Modules/termios.c +++ b/Modules/termios.c @@ -219,7 +219,7 @@ termios_tcsetattr_impl(PyObject *module, int fd, int when, PyObject *term) if (num == -1 && PyErr_Occurred()) { \ return NULL; \ } \ - VAR = (TYPE) num; \ + VAR = (TYPE)num; \ } while (0) SET_FROM_LIST(tcflag_t, mode.c_iflag, term, 0); @@ -246,11 +246,11 @@ termios_tcsetattr_impl(PyObject *module, int fd, int when, PyObject *term) if (PyBytes_Check(v) && PyBytes_Size(v) == 1) mode.c_cc[i] = (cc_t) * PyBytes_AsString(v); else if (PyLong_Check(v)) { - long lng = PyLong_AsLong(v); - if (lng == -1 && PyErr_Occurred()) { + long num = PyLong_AsLong(v); + if (num == -1 && PyErr_Occurred()) { return NULL; } - mode.c_cc[i] = (cc_t) lng; + mode.c_cc[i] = (cc_t)num; } else { PyErr_SetString(PyExc_TypeError,