From fb3fa506ddfe4f36a12736ee74e3b39af17d3b56 Mon Sep 17 00:00:00 2001 From: Ivan Nazarov Date: Mon, 7 Mar 2022 18:34:19 +0300 Subject: [PATCH] Patching sloppy C (#1) * explicit pyTypeObject init, to avoid undupported desginated initializers; fix const correctness in kwlist * fix const correctness in ragged and tools --- src/apply.cpp | 12 ++- src/include/tools.h | 2 +- src/operations.cpp | 14 ++-- src/plyr.cpp | 184 +++++++++++++++++++++++++++++++++----------- src/ragged.cpp | 4 +- src/tools.cpp | 2 +- 6 files changed, 162 insertions(+), 56 deletions(-) diff --git a/src/apply.cpp b/src/apply.cpp index a679cd0..77c01ae 100644 --- a/src/apply.cpp +++ b/src/apply.cpp @@ -533,7 +533,13 @@ PyObject* apply(PyObject *self, PyObject *args, PyObject *kwargs) // handle `apply(..., *, _star, _safe, _finalizer, **kwargs)` // XXX if the finalizer is not required, the kwarg must be omitted. if (kwargs) { - static char *kwlist[] = {"_safe", "_star", "_finalizer", "_strict", NULL}; + static const char *kwlist[] = { + "_safe", + "_star", + "_finalizer", + "_strict", + NULL, + }; // Pop apply's kwargs from `kwargs` so that it could be passed along to // `_apply_base`. @@ -555,7 +561,9 @@ PyObject* apply(PyObject *self, PyObject *args, PyObject *kwargs) // Thus we hold on to the `finalizer` in case its only ref was // the `kwargs`, which we tinkered with just above. int parsed = PyArg_ParseTupleAndKeywords( - empty, own, "|$ppOp:apply", kwlist, &safe, &star, &finalizer, &strict); + empty, own, "|$ppOp:apply", (char**) kwlist, + &safe, &star, &finalizer, &strict + ); Py_DECREF(empty); if (!parsed) { diff --git a/src/include/tools.h b/src/include/tools.h index 00f9bce..6cf51cb 100644 --- a/src/include/tools.h +++ b/src/include/tools.h @@ -5,7 +5,7 @@ PyObject *PyObject_CallWithSingleArg( PyObject *PyDict_SplitItemStrings( PyObject *dict, - char *keys[], + const char *keys[], const bool pop); PyObject* PyTuple_Clone( diff --git a/src/operations.cpp b/src/operations.cpp index f80b494..3e2e500 100644 --- a/src/operations.cpp +++ b/src/operations.cpp @@ -4,11 +4,13 @@ PyObject* getitem(PyObject *self, PyObject *args, PyObject *kwargs) { - static char *kwlist[] = {"", "index", NULL}; + static const char *kwlist[] = {"", "index", NULL}; PyObject *object = NULL, *index = NULL; - int parsed = PyArg_ParseTupleAndKeywords(args, kwargs, "O|$O:getitem", - kwlist, &object, &index); + int parsed = PyArg_ParseTupleAndKeywords( + args, kwargs, "O|$O:getitem", (char**) kwlist, + &object, &index + ); if(!parsed) return NULL; @@ -50,11 +52,13 @@ const PyMethodDef def_xgetitem = { PyObject* setitem(PyObject *self, PyObject *args, PyObject *kwargs) { - static char *kwlist[] = {"", "", "index", NULL}; + static const char *kwlist[] = {"", "", "index", NULL}; PyObject *object = NULL, *value = NULL, *index = NULL; int parsed = PyArg_ParseTupleAndKeywords( - args, kwargs, "OO|$O:setitem", kwlist, &object, &value, &index); + args, kwargs, "OO|$O:setitem", (char**) kwlist, + &object, &value, &index + ); if(!parsed) return NULL; diff --git a/src/plyr.cpp b/src/plyr.cpp index ac4fbed..c0d87a4 100644 --- a/src/plyr.cpp +++ b/src/plyr.cpp @@ -132,90 +132,184 @@ static struct PyModuleDef moduledef = { }; -static PyMethodDef empty_methods[] = { - {NULL}, -}; - - static PyTypeObject AtomicTuple = { PyVarObject_HEAD_INIT(NULL, 0) - .tp_name = "plyr.AtomicTuple", - .tp_doc = PyDoc_STR( + "plyr.AtomicTuple", /* tp_name */ + sizeof(PyTupleObject), /* tp_basicsize */ + 0, /* tp_itemsize */ + 0, /* tp_dealloc */ + 0, /* tp_vectorcall_offset */ + 0, /* tp_getattr */ + 0, /* tp_setattr */ + 0, /* tp_as_async */ + 0, /* tp_repr */ + 0, /* tp_as_number */ + 0, /* tp_as_sequence */ + 0, /* tp_as_mapping */ + 0, /* tp_hash */ + 0, /* tp_call */ + 0, /* tp_str */ + 0, /* tp_getattro */ + 0, /* tp_setattro */ + 0, /* tp_as_buffer */ + (Py_TPFLAGS_DEFAULT + | Py_TPFLAGS_BASETYPE), /* tp_flags */ + PyDoc_STR( "An atomic tuple, NOT considered by plyr as a nested container." - ), - .tp_basicsize = sizeof(PyTupleObject), - .tp_itemsize = 0, - .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, - .tp_init = PyTuple_Type.tp_init, - .tp_methods = empty_methods, + ), /* tp_doc */ + 0, /* tp_traverse */ + 0, /* tp_clear */ + 0, /* tp_richcompare */ + 0, /* tp_weaklistoffset */ + 0, /* tp_iter */ + 0, /* tp_iternext */ + 0, /* tp_methods */ + 0, /* tp_members */ + 0, /* tp_getset */ + 0, // &PyTuple_Type, /* tp_base */ + 0, /* tp_dict */ + 0, /* tp_descr_get */ + 0, /* tp_descr_set */ + 0, /* tp_dictoffset */ + 0, /* tp_init */ + 0, /* tp_alloc */ + 0, /* tp_new */ }; static PyTypeObject AtomicList = { PyVarObject_HEAD_INIT(NULL, 0) - .tp_name = "plyr.AtomicList", - .tp_doc = PyDoc_STR( + "plyr.AtomicList", /* tp_name */ + sizeof(PyListObject), /* tp_basicsize */ + 0, /* tp_itemsize */ + 0, /* tp_dealloc */ + 0, /* tp_vectorcall_offset */ + 0, /* tp_getattr */ + 0, /* tp_setattr */ + 0, /* tp_as_async */ + 0, /* tp_repr */ + 0, /* tp_as_number */ + 0, /* tp_as_sequence */ + 0, /* tp_as_mapping */ + 0, /* tp_hash */ + 0, /* tp_call */ + 0, /* tp_str */ + 0, /* tp_getattro */ + 0, /* tp_setattro */ + 0, /* tp_as_buffer */ + (Py_TPFLAGS_DEFAULT + | Py_TPFLAGS_BASETYPE), /* tp_flags */ + PyDoc_STR( "An atomic list, NOT considered by plyr as a nested container." - ), - .tp_basicsize = sizeof(PyListObject), - .tp_itemsize = 0, - .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, - .tp_init = PyList_Type.tp_init, - .tp_methods = empty_methods, + ), /* tp_doc */ + 0, /* tp_traverse */ + 0, /* tp_clear */ + 0, /* tp_richcompare */ + 0, /* tp_weaklistoffset */ + 0, /* tp_iter */ + 0, /* tp_iternext */ + 0, /* tp_methods */ + 0, /* tp_members */ + 0, /* tp_getset */ + 0, // &PyList_Type, /* tp_base */ + 0, /* tp_dict */ + 0, /* tp_descr_get */ + 0, /* tp_descr_set */ + 0, /* tp_dictoffset */ + 0, /* tp_init */ + 0, /* tp_alloc */ + 0, /* tp_new */ }; static PyTypeObject AtomicDict = { PyVarObject_HEAD_INIT(NULL, 0) - .tp_name = "plyr.AtomicDict", - .tp_doc = PyDoc_STR( + "plyr.AtomicDict", /* tp_name */ + sizeof(PyDictObject), /* tp_basicsize */ + 0, /* tp_itemsize */ + 0, /* tp_dealloc */ + 0, /* tp_vectorcall_offset */ + 0, /* tp_getattr */ + 0, /* tp_setattr */ + 0, /* tp_as_async */ + 0, /* tp_repr */ + 0, /* tp_as_number */ + 0, /* tp_as_sequence */ + 0, /* tp_as_mapping */ + 0, /* tp_hash */ + 0, /* tp_call */ + 0, /* tp_str */ + 0, /* tp_getattro */ + 0, /* tp_setattro */ + 0, /* tp_as_buffer */ + (Py_TPFLAGS_DEFAULT + | Py_TPFLAGS_BASETYPE), /* tp_flags */ + PyDoc_STR( "An atomic dict, NOT considered by plyr as a nested container." - ), - .tp_basicsize = sizeof(PyDictObject), - .tp_itemsize = 0, - .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, - .tp_init = PyDict_Type.tp_init, - .tp_methods = empty_methods, + ), /* tp_doc */ + 0, /* tp_traverse */ + 0, /* tp_clear */ + 0, /* tp_richcompare */ + 0, /* tp_weaklistoffset */ + 0, /* tp_iter */ + 0, /* tp_iternext */ + 0, /* tp_methods */ + 0, /* tp_members */ + 0, /* tp_getset */ + 0, // &PyDict_Type, /* tp_base */ + 0, /* tp_dict */ + 0, /* tp_descr_get */ + 0, /* tp_descr_set */ + 0, /* tp_dictoffset */ + 0, /* tp_init */ + 0, /* tp_alloc */ + 0, /* tp_new */ }; PyMODINIT_FUNC PyInit_plyr(void) { - bool init_failed = false; - - // prepare the leaf container types (excluding namedtuple) + // prepare the atomic leaf container types (excluding namedtuple) + // XXX see the note on `.tp_base` at + // https://docs.python.org/3/c-api/typeobj.html#c.PyTypeObject.tp_base AtomicTuple.tp_base = &PyTuple_Type; - if (PyType_Ready(&AtomicTuple) < 0) - return NULL; - AtomicList.tp_base = &PyList_Type; - if (PyType_Ready(&AtomicList) < 0) - return NULL; - AtomicDict.tp_base = &PyDict_Type; - if (PyType_Ready(&AtomicDict) < 0) + if ( + PyType_Ready(&AtomicTuple) < 0 || + PyType_Ready(&AtomicList) < 0 || + PyType_Ready(&AtomicDict) < 0 + ) return NULL; PyObject *mod = PyModule_Create(&moduledef); if (mod == NULL) return NULL; - /// register custom types (NB AddModule steals refs on success) + bool init_failed = false; + + // register custom types (NB AddModule steals refs on success) Py_INCREF(&AtomicTuple); - Py_INCREF(&AtomicList); - Py_INCREF(&AtomicDict); - if (PyModule_AddObject(mod, "AtomicTuple", (PyObject *) &AtomicTuple) < 0) { + if ( + PyModule_AddObject(mod, "AtomicTuple", (PyObject *) &AtomicTuple) < 0 + ) { Py_DECREF(&AtomicTuple); init_failed = true; } - if (PyModule_AddObject(mod, "AtomicList", (PyObject *) &AtomicList) < 0) { + Py_INCREF(&AtomicList); + if ( + PyModule_AddObject(mod, "AtomicList", (PyObject *) &AtomicList) < 0 + ) { Py_DECREF(&AtomicList); init_failed = true; } - if (PyModule_AddObject(mod, "AtomicDict", (PyObject *) &AtomicDict) < 0) { + Py_INCREF(&AtomicDict); + if ( + PyModule_AddObject(mod, "AtomicDict", (PyObject *) &AtomicDict) < 0 + ) { Py_DECREF(&AtomicDict); init_failed = true; } diff --git a/src/ragged.cpp b/src/ragged.cpp index e25e118..8dee1ff 100644 --- a/src/ragged.cpp +++ b/src/ragged.cpp @@ -371,7 +371,7 @@ PyObject* ragged( return NULL; if (kwargs) { - static char *kwlist[] = {"_star", "_finalizer", NULL}; + static const char *kwlist[] = {"_star", "_finalizer", NULL}; PyObject* own = PyDict_SplitItemStrings(kwargs, kwlist, true); if (own == NULL) { @@ -389,7 +389,7 @@ PyObject* ragged( } int parsed = PyArg_ParseTupleAndKeywords( - empty, own, "|$pO:ragged", kwlist, &star, &finalizer); + empty, own, "|$pO:ragged", (char**) kwlist, &star, &finalizer); Py_DECREF(empty); if (!parsed) { diff --git a/src/tools.cpp b/src/tools.cpp index a421019..f3e98df 100644 --- a/src/tools.cpp +++ b/src/tools.cpp @@ -34,7 +34,7 @@ PyObject *PyObject_CallWithSingleArg( PyObject *PyDict_SplitItemStrings( PyObject *dict, - char *keys[], + const char *keys[], const bool pop=false) { // Pop the specified keys in a NULL-terminated str key list `keys` from