From 9275512f953d1f316591770f29683e40af07bc9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 2 Oct 2024 12:14:26 +0200 Subject: [PATCH 1/6] Improve curses C API cleanup --- Modules/_cursesmodule.c | 173 ++++++++++++++++++++++++++++------------ 1 file changed, 121 insertions(+), 52 deletions(-) diff --git a/Modules/_cursesmodule.c b/Modules/_cursesmodule.c index ece6b13c78851f..72c66627337238 100644 --- a/Modules/_cursesmodule.c +++ b/Modules/_cursesmodule.c @@ -105,8 +105,10 @@ static const char PyCursesVersion[] = "2.2"; #endif #include "Python.h" -#include "pycore_long.h" // _PyLong_GetZero() -#include "pycore_structseq.h" // _PyStructSequence_NewType() +#include "pycore_capsule.h" // _PyCapsule_SetTraverse() +#include "pycore_long.h" // _PyLong_GetZero() +#include "pycore_object.h" // _PyType_HasFeature() +#include "pycore_structseq.h" // _PyStructSequence_NewType() #ifdef __hpux #define STRICT_SYSV_CURSES @@ -626,24 +628,6 @@ class component_converter(CConverter): [python start generated code]*/ /*[python end generated code: output=da39a3ee5e6b4b0d input=38e9be01d33927fb]*/ -/* Function versions of the 3 functions for testing whether curses has been - initialised or not. */ - -static int func_PyCursesSetupTermCalled(void) -{ - return _PyCursesCheckFunction(curses_setupterm_called, "setupterm"); -} - -static int func_PyCursesInitialised(void) -{ - return _PyCursesCheckFunction(curses_initscr_called, "initscr"); -} - -static int func_PyCursesInitialisedColor(void) -{ - return _PyCursesCheckFunction(curses_start_color_called, "start_color"); -} - /***************************************************************************** The Window Object ******************************************************************************/ @@ -4853,29 +4837,108 @@ static PyMethodDef PyCurses_methods[] = { {NULL, NULL} /* sentinel */ }; -/* Initialization function for the module */ +/* Module C API */ +/* Function versions of the 3 functions for testing whether curses has been + initialised or not. */ -static struct PyModuleDef _cursesmodule = { - PyModuleDef_HEAD_INIT, - "_curses", - NULL, - -1, - PyCurses_methods, - NULL, - NULL, - NULL, - NULL -}; +static inline int curses_capi_setupterm_called(void) +{ + return _PyCursesCheckFunction(curses_setupterm_called, "setupterm"); +} + +static inline int curses_capi_initscr_called(void) +{ + return _PyCursesCheckFunction(curses_initscr_called, "initscr"); +} + +static inline int curses_capi_start_color_called(void) +{ + return _PyCursesCheckFunction(curses_start_color_called, "start_color"); +} + +static void * +_curses_capi_new(_cursesmodule_state *state) +{ + assert(state->window_type != NULL); + void **capi = (void **)PyMem_Calloc(PyCurses_API_pointers, sizeof(void *)); + if (capi == NULL) { + PyErr_NoMemory(); + return NULL; + } + capi[0] = (void *)Py_NewRef(state->window_type); + capi[1] = curses_capi_setupterm_called; + capi[2] = curses_capi_initscr_called; + capi[3] = curses_capi_start_color_called; + return (void *)capi; +} static void -curses_destructor(PyObject *op) +_curses_capi_free(void *capi) +{ + assert(capi != NULL); + void **capi_ptr = (void **)capi; + assert(capi_ptr[0] != NULL); + Py_DECREF(capi_ptr[0]); // decref curses window type + PyMem_Free(capi_ptr); +} + +/* Module C API Capsule */ + +static void +_curses_capi_capsule_destructor(PyObject *op) +{ + void *capi = PyCapsule_GetPointer(op, PyCurses_CAPSULE_NAME); + _curses_capi_free(capi); +} + +static int +_curses_capi_capsule_traverse(PyObject *op, visitproc visit, void *arg) +{ + void *capi = PyCapsule_GetPointer(op, PyCurses_CAPSULE_NAME); + assert(capi != NULL); + void *window_type = *(void **)(capi); + assert(window_type != NULL); + if (_PyType_HasFeature(_PyType_CAST(window_type), Py_TPFLAGS_HEAPTYPE)) { + Py_VISIT(window_type); + } + return 0; +} + +static int +_curses_capi_capsule_clear(PyObject *op) { + void *capi = PyCapsule_GetPointer(op, PyCurses_CAPSULE_NAME); + assert(capi != NULL); + void *window_type = *(void **)(capi); + if (_PyType_HasFeature(_PyType_CAST(window_type), Py_TPFLAGS_HEAPTYPE)) { + Py_CLEAR(window_type); + } + return 0; +} + +static PyObject * +_curses_capi_capsule_new(void *capi) { - void *ptr = PyCapsule_GetPointer(op, PyCurses_CAPSULE_NAME); - Py_DECREF(*(void **)ptr); - PyMem_Free(ptr); + PyObject *capsule = PyCapsule_New(capi, PyCurses_CAPSULE_NAME, + _curses_capi_capsule_destructor); + if (capsule == NULL) { + return NULL; + } + void *window_type = *(void **)(capi); + if (_PyType_HasFeature(_PyType_CAST(window_type), Py_TPFLAGS_HEAPTYPE)) { + if (_PyCapsule_SetTraverse(capsule, + _curses_capi_capsule_traverse, + _curses_capi_capsule_clear) < 0) + { + Py_DECREF(capsule); + return NULL; + } + } + return capsule; } +/* Module initialization */ + static int cursesmodule_exec(PyObject *module) { @@ -4895,27 +4958,19 @@ cursesmodule_exec(PyObject *module) return -1; } - void **PyCurses_API = PyMem_Calloc(PyCurses_API_pointers, sizeof(void *)); - if (PyCurses_API == NULL) { - PyErr_NoMemory(); + /* Create the C API object */ + void *capi = _curses_capi_new(state); + if (capi == NULL) { return -1; } - /* Initialize the C API pointer array */ - PyCurses_API[0] = (void *)Py_NewRef(&PyCursesWindow_Type); - PyCurses_API[1] = (void *)func_PyCursesSetupTermCalled; - PyCurses_API[2] = (void *)func_PyCursesInitialised; - PyCurses_API[3] = (void *)func_PyCursesInitialisedColor; - /* Add a capsule for the C API */ - PyObject *c_api_object = PyCapsule_New(PyCurses_API, PyCurses_CAPSULE_NAME, - curses_destructor); - if (c_api_object == NULL) { - Py_DECREF(PyCurses_API[0]); - PyMem_Free(PyCurses_API); + PyObject *capi_capsule = _curses_capi_capsule_new(capi); + if (capi_capsule == NULL) { + _curses_capi_free(capi); return -1; } - int rc = PyDict_SetItemString(module_dict, "_C_API", c_api_object); - Py_DECREF(c_api_object); + int rc = PyDict_SetItemString(module_dict, "_C_API", capi_capsule); + Py_DECREF(capi_capsule); if (rc < 0) { return -1; } @@ -5118,6 +5173,20 @@ cursesmodule_exec(PyObject *module) return 0; } +/* Initialization function for the module */ + +static struct PyModuleDef _cursesmodule = { + PyModuleDef_HEAD_INIT, + "_curses", + NULL, + -1, + PyCurses_methods, + NULL, + NULL, + NULL, + NULL +}; + PyMODINIT_FUNC PyInit__curses(void) { From 42c4c14913a328a545e66cd27156b6cf4237f965 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 2 Oct 2024 17:47:46 +0200 Subject: [PATCH 2/6] remove traverse() and clear() for now --- Modules/_cursesmodule.c | 37 ------------------------------------- 1 file changed, 37 deletions(-) diff --git a/Modules/_cursesmodule.c b/Modules/_cursesmodule.c index 72c66627337238..601140a9737ac4 100644 --- a/Modules/_cursesmodule.c +++ b/Modules/_cursesmodule.c @@ -4892,48 +4892,11 @@ _curses_capi_capsule_destructor(PyObject *op) _curses_capi_free(capi); } -static int -_curses_capi_capsule_traverse(PyObject *op, visitproc visit, void *arg) -{ - void *capi = PyCapsule_GetPointer(op, PyCurses_CAPSULE_NAME); - assert(capi != NULL); - void *window_type = *(void **)(capi); - assert(window_type != NULL); - if (_PyType_HasFeature(_PyType_CAST(window_type), Py_TPFLAGS_HEAPTYPE)) { - Py_VISIT(window_type); - } - return 0; -} - -static int -_curses_capi_capsule_clear(PyObject *op) { - void *capi = PyCapsule_GetPointer(op, PyCurses_CAPSULE_NAME); - assert(capi != NULL); - void *window_type = *(void **)(capi); - if (_PyType_HasFeature(_PyType_CAST(window_type), Py_TPFLAGS_HEAPTYPE)) { - Py_CLEAR(window_type); - } - return 0; -} - static PyObject * _curses_capi_capsule_new(void *capi) { PyObject *capsule = PyCapsule_New(capi, PyCurses_CAPSULE_NAME, _curses_capi_capsule_destructor); - if (capsule == NULL) { - return NULL; - } - void *window_type = *(void **)(capi); - if (_PyType_HasFeature(_PyType_CAST(window_type), Py_TPFLAGS_HEAPTYPE)) { - if (_PyCapsule_SetTraverse(capsule, - _curses_capi_capsule_traverse, - _curses_capi_capsule_clear) < 0) - { - Py_DECREF(capsule); - return NULL; - } - } return capsule; } From dd0c9de9adb8115cece01fec6ec928f9739b0416 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 2 Oct 2024 17:50:22 +0200 Subject: [PATCH 3/6] revert includes --- Modules/_cursesmodule.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Modules/_cursesmodule.c b/Modules/_cursesmodule.c index 601140a9737ac4..b68ae06dec2bcd 100644 --- a/Modules/_cursesmodule.c +++ b/Modules/_cursesmodule.c @@ -105,10 +105,8 @@ static const char PyCursesVersion[] = "2.2"; #endif #include "Python.h" -#include "pycore_capsule.h" // _PyCapsule_SetTraverse() -#include "pycore_long.h" // _PyLong_GetZero() -#include "pycore_object.h" // _PyType_HasFeature() -#include "pycore_structseq.h" // _PyStructSequence_NewType() +#include "pycore_long.h" // _PyLong_GetZero() +#include "pycore_structseq.h" // _PyStructSequence_NewType() #ifdef __hpux #define STRICT_SYSV_CURSES From 8528eb9114ba9d16e13c2a62fb991bc8d63b86cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Thu, 3 Oct 2024 09:25:32 +0200 Subject: [PATCH 4/6] address Victor's review --- Modules/_cursesmodule.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/Modules/_cursesmodule.c b/Modules/_cursesmodule.c index b68ae06dec2bcd..0569362b7fb432 100644 --- a/Modules/_cursesmodule.c +++ b/Modules/_cursesmodule.c @@ -4893,9 +4893,8 @@ _curses_capi_capsule_destructor(PyObject *op) static PyObject * _curses_capi_capsule_new(void *capi) { - PyObject *capsule = PyCapsule_New(capi, PyCurses_CAPSULE_NAME, - _curses_capi_capsule_destructor); - return capsule; + return PyCapsule_New(capi, PyCurses_CAPSULE_NAME, + _curses_capi_capsule_destructor); } /* Module initialization */ @@ -5138,14 +5137,9 @@ cursesmodule_exec(PyObject *module) static struct PyModuleDef _cursesmodule = { PyModuleDef_HEAD_INIT, - "_curses", - NULL, - -1, - PyCurses_methods, - NULL, - NULL, - NULL, - NULL + .m_name = "_curses", + .m_size = -1, + .m_methods = PyCurses_methods, }; PyMODINIT_FUNC From 6a8803fad5ba8536978115fbeb4886895fd7eb95 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Thu, 3 Oct 2024 12:03:49 +0200 Subject: [PATCH 5/6] Apply suggestions from code review --- Modules/_cursesmodule.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Modules/_cursesmodule.c b/Modules/_cursesmodule.c index 0569362b7fb432..88f703dd57f236 100644 --- a/Modules/_cursesmodule.c +++ b/Modules/_cursesmodule.c @@ -4840,17 +4840,20 @@ static PyMethodDef PyCurses_methods[] = { /* Function versions of the 3 functions for testing whether curses has been initialised or not. */ -static inline int curses_capi_setupterm_called(void) +static inline int +curses_capi_setupterm_called(void) { return _PyCursesCheckFunction(curses_setupterm_called, "setupterm"); } -static inline int curses_capi_initscr_called(void) +static inline int +curses_capi_initscr_called(void) { return _PyCursesCheckFunction(curses_initscr_called, "initscr"); } -static inline int curses_capi_start_color_called(void) +static inline int +curses_capi_start_color_called(void) { return _PyCursesCheckFunction(curses_start_color_called, "start_color"); } From d6bed092f2cbf4ea3564998f20648996870d1756 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Thu, 3 Oct 2024 12:24:37 +0200 Subject: [PATCH 6/6] simplify function names --- Modules/_cursesmodule.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/Modules/_cursesmodule.c b/Modules/_cursesmodule.c index 88f703dd57f236..b06c3a862596bb 100644 --- a/Modules/_cursesmodule.c +++ b/Modules/_cursesmodule.c @@ -4859,7 +4859,7 @@ curses_capi_start_color_called(void) } static void * -_curses_capi_new(_cursesmodule_state *state) +curses_capi_new(_cursesmodule_state *state) { assert(state->window_type != NULL); void **capi = (void **)PyMem_Calloc(PyCurses_API_pointers, sizeof(void *)); @@ -4875,7 +4875,7 @@ _curses_capi_new(_cursesmodule_state *state) } static void -_curses_capi_free(void *capi) +curses_capi_free(void *capi) { assert(capi != NULL); void **capi_ptr = (void **)capi; @@ -4887,17 +4887,17 @@ _curses_capi_free(void *capi) /* Module C API Capsule */ static void -_curses_capi_capsule_destructor(PyObject *op) +curses_capi_capsule_destructor(PyObject *op) { void *capi = PyCapsule_GetPointer(op, PyCurses_CAPSULE_NAME); - _curses_capi_free(capi); + curses_capi_free(capi); } static PyObject * -_curses_capi_capsule_new(void *capi) +curses_capi_capsule_new(void *capi) { return PyCapsule_New(capi, PyCurses_CAPSULE_NAME, - _curses_capi_capsule_destructor); + curses_capi_capsule_destructor); } /* Module initialization */ @@ -4922,14 +4922,14 @@ cursesmodule_exec(PyObject *module) } /* Create the C API object */ - void *capi = _curses_capi_new(state); + void *capi = curses_capi_new(state); if (capi == NULL) { return -1; } /* Add a capsule for the C API */ - PyObject *capi_capsule = _curses_capi_capsule_new(capi); + PyObject *capi_capsule = curses_capi_capsule_new(capi); if (capi_capsule == NULL) { - _curses_capi_free(capi); + curses_capi_free(capi); return -1; } int rc = PyDict_SetItemString(module_dict, "_C_API", capi_capsule);