gh-154874: Fix curses.termattrs() returning a negative attribute mask - #154887
Open
mandarwagh9 wants to merge 2 commits into
Open
gh-154874: Fix curses.termattrs() returning a negative attribute mask#154887mandarwagh9 wants to merge 2 commits into
mandarwagh9 wants to merge 2 commits into
Conversation
…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>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
termattrs()returns achtypeattribute mask, but GH-134327 routed it throughNoArgReturnIntFunctionBody, which stores the result in anintso it can be comparedagainst
ERR. On a terminal that advertisesA_ITALIC— the topmost bit of a 32-bitmask — that sign-extends, and the mask comes back negative:
The value is now returned unsigned, the same way
term_attrs(),slk_attr()andwindow.getattrs()already do. The cast goes throughchtyperather than straight tounsigned long, matchinggetattrs(), so no sign extension can happen on a platformthat declares
termattrs()as returning anint.baudrate()is the macro's only other user and really does return a status, so it keepsthe
ERRcheck. 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
TermAttrsTestsdrivesnewterm('xterm-256color')over a pseudo-terminal rather thanrelying on
$TERM, which on CI is usuallylinuxand advertises no italic. Its twoassertions 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) givesmainand3.15only,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
afterwards. Happy for whichever is more useful to maintainers to land — closing this
one is completely fine.
_curseslocally (Windows), so the test here is unverifiedby me and I am relying on CI to run it. Worth a careful look on that account.
me, but please weigh the point above accordingly.