Skip to content

gh-154874: Fix curses.termattrs() returning a negative attribute mask - #154887

Closed
mandarwagh9 wants to merge 2 commits into
python:mainfrom
mandarwagh9:fix-termattrs-sign
Closed

gh-154874: Fix curses.termattrs() returning a negative attribute mask#154887
mandarwagh9 wants to merge 2 commits into
python:mainfrom
mandarwagh9:fix-termattrs-sign

Conversation

@mandarwagh9

Copy link
Copy Markdown

termattrs() returns a chtype attribute mask, but GH-134327 routed it through
NoArgReturnIntFunctionBody, which stores the result in an int so it can be compared
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:

>>> curses.termattrs()
-2130771968
>>> curses.term_attrs()          # the same bits, unsigned
2164195328
>>> curses.newwin(1, 1).attrset(curses.termattrs())
OverflowError: can't convert negative value to unsigned int

The value is now returned unsigned, the same way term_attrs(), slk_attr() and
window.getattrs() already do. The cast goes through chtype rather than straight to
unsigned long, matching getattrs(), so no sign extension can happen on a platform
that declares termattrs() as returning an int.

baudrate() is the macro's only other user and really does return a status, so it keeps
the ERR check. The macro gains a short comment saying it is only for such functions —
the same mistake is easy to repeat, and an all-bits-set mask would additionally be
misread as ERR.

Test

TermAttrsTests drives newterm('xterm-256color') over a pseudo-terminal rather than
relying on $TERM, which on CI is usually linux and advertises no italic. Its two
assertions hold on any terminal, so it never skips for lack of an italic-capable one; it
fails without the C change wherever the top bit is actually advertised.

Backport

git branch -r --contains 30dde1ee (the GH-134327 merge) gives main and 3.15 only,
so this needs a 3.15 backport and nothing older. 3.14 is unaffected. The issue carries
3.14 and 3.16 labels as well, but 3.14 predates the regression.

Notes

  • gh-154874: Fix the sign of curses.termattrs() #154875 proposes the same one-line fix; I opened this independently and only saw it
    afterwards. Happy for whichever is more useful to maintainers to land — closing this
    one is completely fine.
  • I could not build or run _curses locally (Windows), so the test here is unverified
    by me
    and I am relying on CI to run it. Worth a careful look on that account.
  • Prepared with AI assistance (Claude); the reasoning and the diff have been reviewed by
    me, but please weigh the point above accordingly.

…e mask

termattrs() returns a chtype mask, but pythonGH-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>
Copilot AI review requested due to automatic review settings July 29, 2026 16:35

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@picnixz picnixz closed this Jul 30, 2026
@picnixz

picnixz commented Jul 30, 2026

Copy link
Copy Markdown
Member

There is already an open PR. Do not duplicate existing work especially not with just agents

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants