8000 Exact reporting of `curses` C function failures · Issue #133579 · python/cpython · GitHub
[go: up one dir, main page]

Skip to content

Exact reporting of curses C function failures #133579

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
picnixz opened this issue May 7, 2025 · 3 comments
Open

Exact reporting of curses C function failures #133579

picnixz opened this issue May 7, 2025 · 3 comments
Assignees
Labels
extension-modules C modules in the Modules dir type-bug An unexpected behavior, bug, or error

Comments

@picnixz
Copy link
Member
picnixz commented May 7, 2025

When calling some curses C functions, we ignore their return values. Those are:

  • in PyCursesWindow_dealloc, we ignore delwin() errors

  • in _curses_initscr_impl, we ignore wrefresh() errors

  • in _curses_window_addstr_impl, we ignore wattrset() errors and prefer reporting errors due to adding strings. Same for the insstr* functions

  • in _curses_window_box_impl, we ignore the error returned by box(). This is correct because the manual says (for box):

    All routines return the integer OK. The SVr4.0 manual says "or a non-negative integer if immedok is set", but this appears to be an error. X/Open does not define any error conditions. This implementation returns an error if the window pointer is null.

    In this case, I think it's better to explicitly suppress the return value and link the manpage.

The question now is:

  • What should we do for a failing delwin() in the destructor?
  • What should we do for wattrset() when this routine is not the "important" one?

cc @encukou


Note

We will not backport these fixes as they could break existing code that could be surprised by a new exception.

Linked PRs

@picnixz picnixz self-assigned this May 7, 2025
@picnixz picnixz added type-feature A feature request or enhancement extension-modules C modules in the Modules dir labels May 7, 2025
@encukou
Copy link
Member
encukou commented May 17, 2025

What should we do for a failing delwin() in the destructor?

Set an exception and call PyErr_WriteUnraisable, so that it's caught by unraisablehook.

What should we do for wattrset() when this routine is not the "important" one?

In _curses_window_addstr_impl, wattrset is only called if the user explicitly called addstr with an attr argument. Failure to set them should be a hard error, to be handled by the user.

_curses_window_box_impl

IMO, if an error is reported, something is wrong and we should raise an exception.


IMO, these would be bugfixes, but should not be backported.

@picnixz picnixz added type-bug An unexpected behavior, bug, or error triaged The issue has been accepted as valid by a triager. and removed type-feature A feature request or enhancement triaged The issue has been accepted as valid by a triager. labels May 18, 2025
@picnixz
Copy link
Member Author
picnixz commented May 18, 2025

Thanks. I've found other occurrences where curses C functions are called but the error handling is not done. It's just that it was sometimes under ifdef that I missed. I need #125844 to be merged first so that I can use the helpers out there so I'm holding this work for now.

@picnixz
Copy link
Member Author
picnixz commented May 19, 2025

On failure, _curses.window.getstr also return an empty string instead of reporting the error. It may be fine, but I don't know if it wouldn't be better to actually report the possible error.

picnixz added a commit that referenced this issue May 27, 2025
Some curses module-level functions and window methods now raise
a `curses.error` when a call to a C curses function fails:

- Module-level functions: assume_default_colors, baudrate, cbreak,
  echo, longname, initscr, nl, raw, termattrs, termname, and unctrl.
- Window methods: addch, addnstr, addstr, border, box, chgat,
  getbkgd, inch, insstr, and insnstr.

In addition, `curses.window.refresh` and `curses.window.noutrefresh`
now raise a `TypeError` instead of a `curses.error` when called with an
incorrect number of arguments for pads.

See also ee36db5 for similar
changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extension-modules C modules in the Modules dir type-bug An unexpected behavior, bug, or error
Projects
Status: No status
Development

No branches or pull requests

2 participants
0