Skip to content

Commit da6982e

Browse files
Mandar Waghclaude
andcommitted
gh-154874: Fix curses.termattrs() returning a negative attribute mask
termattrs() returns a chtype mask, but GH-134327 routed it through the NoArgReturnIntFunctionBody macro, which stores the result in an int to compare it against ERR. On a terminal that advertises A_ITALIC, the topmost bit of a 32-bit mask, that sign-extends and the mask comes back negative, so it can no longer be passed to the attribute functions: >>> curses.termattrs() -2130771968 >>> curses.newwin(1, 1).attrset(curses.termattrs()) OverflowError: can't convert negative value to unsigned int Return the mask unsigned instead, the same way term_attrs(), slk_attr() and window.getattrs() already do. baudrate(), the macro's only other user, really does return a status, so it keeps the ERR check; the macro gains a comment saying it is only for such functions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent bfc16a7 commit da6982e

3 files changed

Lines changed: 43 additions & 1 deletion

File tree

Lib/test/test_curses.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3242,5 +3242,32 @@ def test_color(self):
32423242
curses.slk_color(0)
32433243

32443244

3245+
@unittest.skipUnless(hasattr(curses, 'newterm'), 'requires curses.newterm()')
3246+
@unittest.skipIf(BROKEN_NEWTERM, 'ncurses < 6.5 mishandles repeated newterm()')
3247+
@unittest.skipIf(not term or term == 'unknown',
3248+
f"$TERM={term!r}, newterm() may not work")
3249+
@unittest.skipIf(sys.platform == "cygwin",
3250+
"cygwin's curses mostly just hangs")
3251+
class TermAttrsTests(NewtermTestBase):
3252+
# A signed termattrs() only differs from an unsigned one on a terminal
3253+
# that advertises the topmost bit of the mask, which is A_ITALIC. Drive
3254+
# a terminal type that supports italics over a pseudo-terminal instead of
3255+
# trusting $TERM, which on CI is usually 'linux' and advertises none.
3256+
3257+
def test_termattrs_is_a_usable_mask(self):
3258+
s = self.make_pty()
3259+
try:
3260+
screen = curses.newterm('xterm-256color', s, s)
3261+
except curses.error:
3262+
self.skipTest('no xterm-256color terminfo entry')
3263+
attrs = curses.termattrs()
3264+
# An attribute mask is unsigned, whichever attributes the terminal
3265+
# happens to support.
3266+
self.assertGreaterEqual(attrs, 0)
3267+
# termattrs() exists to be passed back to the attribute functions,
3268+
# which reject a negative mask with OverflowError.
3269+
screen.stdscr.attrset(attrs)
3270+
3271+
32453272
if __name__ == '__main__':
32463273
unittest.main()
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix :func:`curses.termattrs` returning a negative value on a terminal that
2+
supports :const:`curses.A_ITALIC`. The attribute mask is now returned
3+
unsigned, so it can be passed back to the attribute functions.

Modules/_cursesmodule.c

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5495,6 +5495,10 @@ static PyType_Spec PyCursesScreen_Type_spec = {
54955495
return curses_check_err(module, rtn, funcname, # X); \
54965496
}
54975497

5498+
/* Only for a function returning an int that may be ERR. A function returning
5499+
an attribute mask must not use this: the mask is unsigned and its topmost
5500+
bit is a valid attribute, so storing it in an int makes the result both
5501+
negative and, for an all-bits-set mask, indistinguishable from ERR. */
54985502
#define NoArgReturnIntFunctionBody(X) \
54995503
{ \
55005504
PyCursesStatefulInitialised(module); \
@@ -7899,7 +7903,15 @@ Return a logical OR of all video attributes supported by the terminal.
78997903
static PyObject *
79007904
_curses_termattrs_impl(PyObject *module)
79017905
/*[clinic end generated code: output=b06f437fce1b6fc4 input=0559882a04f84d1d]*/
7902-
NoArgReturnIntFunctionBody(termattrs)
7906+
{
7907+
PyCursesStatefulInitialised(module);
7908+
7909+
/* termattrs() returns a chtype mask, not a status, so there is no ERR to
7910+
check for. Go through chtype rather than int: A_ITALIC is the topmost
7911+
bit of a 32-bit mask, and sign-extending it would make the result
7912+
negative and unusable as an attribute. */
7913+
return PyLong_FromUnsignedLong((unsigned long)(chtype)termattrs());
7914+
}
79037915

79047916
#ifdef HAVE_CURSES_TERM_ATTRS
79057917
/*[clinic input]

0 commit comments

Comments
 (0)