Skip to content
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

gh-123961: move PyCursesError in _cursesmodule.c to a global state variable #124729

Merged
merged 39 commits into from
Sep 29, 2024
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
3148d86
Introduce module's state and logic.
picnixz Sep 28, 2024
75bd7b6
Store exception in the module's state.
picnixz Sep 28, 2024
b295919
Rename `curses_destructor` to `_curses_capi_destructor`
picnixz Sep 28, 2024
1665f38
Introduce macros for initialization checks.
picnixz Sep 28, 2024
2a1e13a
Update C API functions
picnixz Sep 28, 2024
9e29bd5
Update macros checking the functions being called.
picnixz Sep 28, 2024
e1cab2f
Update `PyCursesCheckERR` to use the module's state
picnixz Sep 28, 2024
5f28eb8
Add `PyCursesCheckERR_FromSelf` to prepare when the type is a heap type.
picnixz Sep 28, 2024
bcbbdd4
Update Function Prototype Macros.
picnixz Sep 28, 2024
faf4e87
Update Function Body Macros.
picnixz Sep 28, 2024
885c94c
Update `PyCursesCheckERR` usage for Window methods
picnixz Sep 28, 2024
da98de4
Update `PyCursesCheckERR` usage for module's methods
picnixz Sep 28, 2024
65c5f60
Update global references to `PyCursesError`
picnixz Sep 28, 2024
e3e1131
Make `PyCursesWindow_Type` a heap type.
picnixz Sep 28, 2024
9617f50
Correct calls to `PyCursesWindow_New`
picnixz Sep 28, 2024
1db7bea
Do not expose `PyCursesWindow_Check` when compiling `_cursesmodule.c`
picnixz Sep 28, 2024
4c207d3
Finalize multi-phase initialization for `curses`.
picnixz Sep 28, 2024
228348c
Remove redundant cast to `PyObject *`.
picnixz Sep 28, 2024
b8b0751
reduce diff (remove some PEP-7 changes)
picnixz Sep 28, 2024
83fab15
reduce diff (remove some PEP-7 changes)
picnixz Sep 28, 2024
9ff69e4
Revert "Finalize multi-phase initialization for `curses`."
picnixz Sep 28, 2024
13bd868
Revert "Introduce module's state and logic."
picnixz Sep 28, 2024
f32dee9
Revert "Make `PyCursesWindow_Type` a heap type."
picnixz Sep 28, 2024
fd38850
address Victor's review
picnixz Sep 28, 2024
e70175c
Revert "Remove redundant cast to `PyObject *`."
picnixz Sep 28, 2024
676d1ce
reduce diff
picnixz Sep 28, 2024
8a60d53
reduce diff
picnixz Sep 28, 2024
e0dfc4a
fix globals
picnixz Sep 28, 2024
e596924
fix globals
picnixz Sep 28, 2024
fc9dacb
rename the state fields
picnixz Sep 29, 2024
f548f4b
Introduce macros for initialization checks.
picnixz Sep 29, 2024
3e5c37c
Update C API functions
picnixz Sep 29, 2024
b94438f
Improve macros to use module's state when possible
picnixz Sep 29, 2024
a4edf7e
Merge branch 'curses/multiphase-module-123961' of github.com:picnixz/…
picnixz Sep 29, 2024
b987499
Make use of module's state whenever possible.
picnixz Sep 29, 2024
e8d9d66
Make use of module's state whenever possible.
picnixz Sep 29, 2024
b02e860
remove unused function for now
picnixz Sep 29, 2024
439a63f
address Victor's review
picnixz Sep 29, 2024
3936143
fix test
picnixz Sep 29, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Include/py_curses.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,6 @@ typedef struct {
char *encoding;
} PyCursesWindowObject;

#define PyCursesWindow_Check(v) Py_IS_TYPE((v), &PyCursesWindow_Type)

#define PyCurses_CAPSULE_NAME "_curses._C_API"


Expand All @@ -99,6 +97,8 @@ static void **PyCurses_API;
#define PyCursesInitialised {if (! ((int (*)(void))PyCurses_API[2]) () ) return NULL;}
#define PyCursesInitialisedColor {if (! ((int (*)(void))PyCurses_API[3]) () ) return NULL;}

#define PyCursesWindow_Check(v) Py_IS_TYPE((v), &PyCursesWindow_Type)

#define import_curses() \
PyCurses_API = (void **)PyCapsule_Import(PyCurses_CAPSULE_NAME, 1);

Expand Down
107 changes: 65 additions & 42 deletions Modules/_cursesmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -159,16 +159,26 @@ typedef chtype attr_t; /* No attr_t type is available */
#define _CURSES_PAIR_CONTENT_FUNC pair_content
#endif /* _NCURSES_EXTENDED_COLOR_FUNCS */

typedef struct _cursesmodule_state {
PyObject *PyCursesError;
PyTypeObject *PyCursesWindow_Type;
picnixz marked this conversation as resolved.
Show resolved Hide resolved
} _cursesmodule_state;

// For now, we keep a global state variable to prepare for PEP 489.
static _cursesmodule_state curses_global_state;

static inline _cursesmodule_state *
get_cursesmodule_state(PyObject *Py_UNUSED(module))
{
return &curses_global_state;
}

/*[clinic input]
module _curses
class _curses.window "PyCursesWindowObject *" "&PyCursesWindow_Type"
[clinic start generated code]*/
/*[clinic end generated code: output=da39a3ee5e6b4b0d input=43265c372c2887d6]*/

/* Definition of exception curses.error */

static PyObject *PyCursesError;

/* Tells whether setupterm() has been called to initialise terminfo. */
static int curses_setupterm_called = FALSE;

Expand All @@ -184,25 +194,25 @@ static const char *curses_screen_encoding = NULL;
#define PyCursesSetupTermCalled \
do { \
if (curses_setupterm_called != TRUE) { \
PyErr_SetString(PyCursesError, \
PyErr_SetString(curses_global_state.PyCursesError, \
picnixz marked this conversation as resolved.
Show resolved Hide resolved
"must call (at least) setupterm() first"); \
return 0; \
} \
} while (0)

#define PyCursesInitialised \
do { \
if (curses_initscr_called != TRUE) { \
PyErr_SetString(PyCursesError, \
"must call initscr() first"); \
return 0; \
} \
#define PyCursesInitialised \
do { \
if (curses_initscr_called != TRUE) { \
PyErr_SetString(curses_global_state.PyCursesError, \
"must call initscr() first"); \
return 0; \
} \
} while (0)

#define PyCursesInitialisedColor \
do { \
if (curses_start_color_called != TRUE) { \
PyErr_SetString(PyCursesError, \
PyErr_SetString(curses_global_state.PyCursesError, \
"must call start_color() first"); \
return 0; \
} \
Expand All @@ -223,9 +233,9 @@ PyCursesCheckERR(int code, const char *fname)
Py_RETURN_NONE;
} else {
if (fname == NULL) {
PyErr_SetString(PyCursesError, catchall_ERR);
PyErr_SetString(curses_global_state.PyCursesError, catchall_ERR);
} else {
PyErr_Format(PyCursesError, "%s() returned ERR", fname);
PyErr_Format(curses_global_state.PyCursesError, "%s() returned ERR", fname);
}
return NULL;
}
Expand Down Expand Up @@ -1331,7 +1341,7 @@ _curses_window_derwin_impl(PyCursesWindowObject *self, int group_left_1,
win = derwin(self->win,nlines,ncols,begin_y,begin_x);

if (win == NULL) {
PyErr_SetString(PyCursesError, catchall_NULL);
PyErr_SetString(curses_global_state.PyCursesError, catchall_NULL);
return NULL;
}

Expand Down Expand Up @@ -1481,7 +1491,7 @@ _curses_window_getkey_impl(PyCursesWindowObject *self, int group_right_1,
/* getch() returns ERR in nodelay mode */
PyErr_CheckSignals();
if (!PyErr_Occurred())
PyErr_SetString(PyCursesError, "no input");
PyErr_SetString(curses_global_state.PyCursesError, "no input");
return NULL;
} else if (rtn <= 255) {
#ifdef NCURSES_VERSION_MAJOR
Expand Down Expand Up @@ -1539,7 +1549,7 @@ _curses_window_get_wch_impl(PyCursesWindowObject *self, int group_right_1,
return NULL;

/* get_wch() returns ERR in nodelay mode */
PyErr_SetString(PyCursesError, "no input");
PyErr_SetString(curses_global_state.PyCursesError, "no input");
return NULL;
}
if (ct == KEY_CODE_YES)
Expand Down Expand Up @@ -2052,7 +2062,7 @@ _curses_window_noutrefresh_impl(PyCursesWindowObject *self)
#ifdef py_is_pad
if (py_is_pad(self->win)) {
if (!group_right_1) {
PyErr_SetString(PyCursesError,
PyErr_SetString(curses_global_state.PyCursesError,
"noutrefresh() called for a pad "
"requires 6 arguments");
return NULL;
Expand Down Expand Up @@ -2276,7 +2286,7 @@ _curses_window_refresh_impl(PyCursesWindowObject *self, int group_right_1,
#ifdef py_is_pad
if (py_is_pad(self->win)) {
if (!group_right_1) {
PyErr_SetString(PyCursesError,
PyErr_SetString(curses_global_state.PyCursesError,
"refresh() for a pad requires 6 arguments");
return NULL;
}
Expand Down Expand Up @@ -2358,7 +2368,7 @@ _curses_window_subwin_impl(PyCursesWindowObject *self, int group_left_1,
win = subwin(self->win, nlines, ncols, begin_y, begin_x);

if (win == NULL) {
PyErr_SetString(PyCursesError, catchall_NULL);
PyErr_SetString(curses_global_state.PyCursesError, catchall_NULL);
return NULL;
}

Expand Down Expand Up @@ -2632,7 +2642,7 @@ PyTypeObject PyCursesWindow_Type = {
PyCursesWindow_getsets, /* tp_getset */
};

/* Function Prototype Macros - They are ugly but very, very useful. ;-)
/* Function Body Macros - They are ugly but very, very useful. ;-)

X - function name
TYPE - parameter Type
Expand Down Expand Up @@ -2774,8 +2784,9 @@ _curses_color_content_impl(PyObject *module, int color_number)
PyCursesInitialisedColor;

if (_COLOR_CONTENT_FUNC(color_number, &r, &g, &b) == ERR) {
PyErr_Format(PyCursesError, "%s() returned ERR",
Py_STRINGIFY(_COLOR_CONTENT_FUNC));
_cursesmodule_state *st = get_cursesmodule_state(module);
PyErr_Format(st->PyCursesError, "%s() returned ERR",
Py_STRINGIFY(_COLOR_CONTENT_FUNC));
return NULL;
}

Expand Down Expand Up @@ -3013,7 +3024,8 @@ _curses_getmouse_impl(PyObject *module)

rtn = getmouse( &event );
if (rtn == ERR) {
PyErr_SetString(PyCursesError, "getmouse() returned ERR");
_cursesmodule_state *st = get_cursesmodule_state(module);
PyErr_SetString(st->PyCursesError, "getmouse() returned ERR");
return NULL;
}
return Py_BuildValue("(hiiik)",
Expand Down Expand Up @@ -3107,7 +3119,8 @@ _curses_getwin(PyObject *module, PyObject *file)
fseek(fp, 0, 0);
win = getwin(fp);
if (win == NULL) {
PyErr_SetString(PyCursesError, catchall_NULL);
_cursesmodule_state *st = get_cursesmodule_state(module);
PyErr_SetString(st->PyCursesError, catchall_NULL);
goto error;
}
res = PyCursesWindow_New(win, NULL);
Expand Down Expand Up @@ -3255,7 +3268,8 @@ _curses_init_pair_impl(PyObject *module, int pair_number, int fg, int bg)
COLOR_PAIRS - 1);
}
else {
PyErr_Format(PyCursesError, "%s() returned ERR",
_cursesmodule_state *st = get_cursesmodule_state(module);
PyErr_Format(st->PyCursesError, "%s() returned ERR",
Py_STRINGIFY(_CURSES_INIT_PAIR_FUNC));
}
return NULL;
Expand Down Expand Up @@ -3286,7 +3300,8 @@ _curses_initscr_impl(PyObject *module)
win = initscr();

if (win == NULL) {
PyErr_SetString(PyCursesError, catchall_NULL);
_cursesmodule_state *st = get_cursesmodule_state(module);
PyErr_SetString(st->PyCursesError, catchall_NULL);
return NULL;
}

Expand Down Expand Up @@ -3415,9 +3430,8 @@ _curses_setupterm_impl(PyObject *module, const char *term, int fd)
sys_stdout = PySys_GetObject("stdout");

if (sys_stdout == NULL || sys_stdout == Py_None) {
PyErr_SetString(
PyCursesError,
"lost sys.stdout");
_cursesmodule_state *st = get_cursesmodule_state(module);
PyErr_SetString(st->PyCursesError, "lost sys.stdout");
return NULL;
}

Expand All @@ -3437,7 +3451,8 @@ _curses_setupterm_impl(PyObject *module, const char *term, int fd)
s = "setupterm: could not find terminfo database";
}

PyErr_SetString(PyCursesError,s);
_cursesmodule_state *st = get_cursesmodule_state(module);
PyErr_SetString(st->PyCursesError, s);
return NULL;
}

Expand Down Expand Up @@ -3754,7 +3769,8 @@ _curses_newpad_impl(PyObject *module, int nlines, int ncols)
win = newpad(nlines, ncols);

if (win == NULL) {
PyErr_SetString(PyCursesError, catchall_NULL);
_cursesmodule_state *st = get_cursesmodule_state(module);
PyErr_SetString(st->PyCursesError, catchall_NULL);
return NULL;
}

Expand Down Expand Up @@ -3793,7 +3809,8 @@ _curses_newwin_impl(PyObject *module, int nlines, int ncols,

win = newwin(nlines,ncols,begin_y,begin_x);
if (win == NULL) {
PyErr_SetString(PyCursesError, catchall_NULL);
_cursesmodule_state *st = get_cursesmodule_state(module);
PyErr_SetString(st->PyCursesError, catchall_NULL);
return NULL;
}

Expand Down Expand Up @@ -3911,7 +3928,8 @@ _curses_pair_content_impl(PyObject *module, int pair_number)
COLOR_PAIRS - 1);
}
else {
PyErr_Format(PyCursesError, "%s() returned ERR",
_cursesmodule_state *st = get_cursesmodule_state(module);
PyErr_Format(st->PyCursesError, "%s() returned ERR",
Py_STRINGIFY(_CURSES_PAIR_CONTENT_FUNC));
}
return NULL;
Expand Down Expand Up @@ -4241,7 +4259,8 @@ _curses_start_color_impl(PyObject *module)
PyCursesInitialised;

if (start_color() == ERR) {
PyErr_SetString(PyCursesError, "start_color() returned ERR");
_cursesmodule_state *st = get_cursesmodule_state(module);
PyErr_SetString(st->PyCursesError, "start_color() returned ERR");
return NULL;
}

Expand Down Expand Up @@ -4393,7 +4412,8 @@ _curses_tparm_impl(PyObject *module, const char *str, int i1, int i2, int i3,

result = tparm((char *)str,i1,i2,i3,i4,i5,i6,i7,i8,i9);
if (!result) {
PyErr_SetString(PyCursesError, "tparm() returned NULL");
_cursesmodule_state *st = get_cursesmodule_state(module);
PyErr_SetString(st->PyCursesError, "tparm() returned NULL");
return NULL;
}

Expand Down Expand Up @@ -4594,7 +4614,8 @@ _curses_use_default_colors_impl(PyObject *module)
if (code != ERR) {
Py_RETURN_NONE;
} else {
PyErr_SetString(PyCursesError, "use_default_colors() returned ERR");
_cursesmodule_state *st = get_cursesmodule_state(module);
PyErr_SetString(st->PyCursesError, "use_default_colors() returned ERR");
return NULL;
}
}
Expand Down Expand Up @@ -4785,13 +4806,15 @@ curses_destructor(PyObject *op)
static int
cursesmodule_exec(PyObject *module)
{
_cursesmodule_state *st = get_cursesmodule_state(module);
/* Initialize object type */
if (PyType_Ready(&PyCursesWindow_Type) < 0) {
return -1;
}
if (PyModule_AddType(module, &PyCursesWindow_Type) < 0) {
return -1;
}
st->PyCursesWindow_Type = &PyCursesWindow_Type;

/* Add some symbolic constants to the module */
PyObject *module_dict = PyModule_GetDict(module);
Expand Down Expand Up @@ -4825,12 +4848,12 @@ cursesmodule_exec(PyObject *module)
}

/* For exception curses.error */
PyCursesError = PyErr_NewException("_curses.error", NULL, NULL);
if (PyCursesError == NULL) {
st->PyCursesError = PyErr_NewException("_curses.error", NULL, NULL);
if (st->PyCursesError == NULL) {
return -1;
}
rc = PyDict_SetItemString(module_dict, "error", PyCursesError);
Py_DECREF(PyCursesError);
rc = PyDict_SetItemString(module_dict, "error", st->PyCursesError);
Py_DECREF(st->PyCursesError);
if (rc < 0) {
return -1;
}
Expand Down
1 change: 1 addition & 0 deletions Tools/c-analyzer/cpython/globals-to-fix.tsv
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,7 @@ Modules/readline.c - libedit_history_start -

Modules/_ctypes/cfield.c - formattable -
Modules/_ctypes/malloc_closure.c - free_list -
Modules/_cursesmodule.c - curses_global_state -
Modules/_curses_panel.c - lop -
Modules/_ssl/debughelpers.c _PySSL_keylog_callback lock -
Modules/_tkinter.c - quitMainLoop -
Expand Down
Loading