From c98600bed47b0be4d9d601c78252154118e7366f Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Tue, 14 Nov 2023 11:28:34 +0200 Subject: gh-111789: Use PyDict_GetItemRef() in _ctypes (GH-111828) --- Modules/_ctypes/_ctypes.c | 104 ++++++++++++++++++++------------------------- Modules/_ctypes/callproc.c | 35 +++++++-------- 2 files changed, 61 insertions(+), 78 deletions(-) diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index 6e0709c..f909a94 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -243,26 +243,13 @@ PyDict_SetItemProxy(PyObject *dict, PyObject *key, PyObject *item) static int _PyDict_GetItemProxy(PyObject *dict, PyObject *key, PyObject **presult) { - PyObject *item = PyDict_GetItemWithError(dict, key); - if (item == NULL) { - if (PyErr_Occurred()) { - return -1; - } - *presult = NULL; - return 0; + int rc = PyDict_GetItemRef(dict, key, presult); + PyObject *item = *presult; + if (item && PyWeakref_CheckProxy(item)) { + rc = PyWeakref_GetRef(item, presult); + Py_DECREF(item); } - - if (!PyWeakref_CheckProxy(item)) { - *presult = Py_NewRef(item); - return 0; - } - PyObject *ref; - if (PyWeakref_GetRef(item, &ref) < 0) { - return -1; - } - // ref is NULL if the referenced object was destroyed - *presult = ref; - return 0; + return rc; } /******************************************************************/ @@ -565,18 +552,19 @@ StructUnionType_new(PyTypeObject *type, PyObject *args, PyObject *kwds, int isSt dict->paramfunc = StructUnionType_paramfunc; - fields = PyDict_GetItemWithError((PyObject *)dict, &_Py_ID(_fields_)); + if (PyDict_GetItemRef((PyObject *)dict, &_Py_ID(_fields_), &fields) < 0) { + Py_DECREF(result); + return NULL; + } if (fields) { if (PyObject_SetAttr((PyObject *)result, &_Py_ID(_fields_), fields) < 0) { Py_DECREF(result); + Py_DECREF(fields); return NULL; } + Py_DECREF(fields); return (PyObject *)result; } - else if (PyErr_Occurred()) { - Py_DECREF(result); - return NULL; - } else { StgDictObject *basedict = PyType_stgdict((PyObject *)result->tp_base); @@ -1110,11 +1098,15 @@ PyCPointerType_new(PyTypeObject *type, PyObject *args, PyObject *kwds) stgdict->paramfunc = PyCPointerType_paramfunc; stgdict->flags |= TYPEFLAG_ISPOINTER; - proto = PyDict_GetItemWithError(typedict, &_Py_ID(_type_)); /* Borrowed ref */ + if (PyDict_GetItemRef(typedict, &_Py_ID(_type_), &proto) < 0) { + Py_DECREF((PyObject *)stgdict); + return NULL; + } if (proto) { StgDictObject *itemdict; const char *current_format; if (-1 == PyCPointerType_SetProto(stgdict, proto)) { + Py_DECREF(proto); Py_DECREF((PyObject *)stgdict); return NULL; } @@ -1134,15 +1126,12 @@ PyCPointerType_new(PyTypeObject *type, PyObject *args, PyObject *kwds) } else { stgdict->format = _ctypes_alloc_format_string("&", current_format); } + Py_DECREF(proto); if (stgdict->format == NULL) { Py_DECREF((PyObject *)stgdict); return NULL; } } - else if (PyErr_Occurred()) { - Py_DECREF((PyObject *)stgdict); - return NULL; - } /* create the new instance (which is a class, since we are a metatype!) */ @@ -2461,58 +2450,61 @@ make_funcptrtype_dict(StgDictObject *stgdict) stgdict->getfunc = NULL; stgdict->ffi_type_pointer = ffi_type_pointer; - ob = PyDict_GetItemWithError((PyObject *)stgdict, &_Py_ID(_flags_)); + if (PyDict_GetItemRef((PyObject *)stgdict, &_Py_ID(_flags_), &ob) < 0) { + return -1; + } if (!ob || !PyLong_Check(ob)) { - if (!PyErr_Occurred()) { - PyErr_SetString(PyExc_TypeError, + PyErr_SetString(PyExc_TypeError, "class must define _flags_ which must be an integer"); - } + Py_XDECREF(ob); return -1; } stgdict->flags = PyLong_AsUnsignedLongMask(ob) | TYPEFLAG_ISPOINTER; + Py_DECREF(ob); /* _argtypes_ is optional... */ - ob = PyDict_GetItemWithError((PyObject *)stgdict, &_Py_ID(_argtypes_)); + if (PyDict_GetItemRef((PyObject *)stgdict, &_Py_ID(_argtypes_), &ob) < 0) { + return -1; + } if (ob) { converters = converters_from_argtypes(ob); - if (!converters) + if (!converters) { + Py_DECREF(ob); return -1; - stgdict->argtypes = Py_NewRef(ob); + } + stgdict->argtypes = ob; stgdict->converters = converters; } - else if (PyErr_Occurred()) { + + if (PyDict_GetItemRef((PyObject *)stgdict, &_Py_ID(_restype_), &ob) < 0) { return -1; } - - ob = PyDict_GetItemWithError((PyObject *)stgdict, &_Py_ID(_restype_)); if (ob) { if (ob != Py_None && !PyType_stgdict(ob) && !PyCallable_Check(ob)) { PyErr_SetString(PyExc_TypeError, "_restype_ must be a type, a callable, or None"); + Py_DECREF(ob); return -1; } - stgdict->restype = Py_NewRef(ob); + stgdict->restype = ob; if (PyObject_GetOptionalAttr(ob, &_Py_ID(_check_retval_), &stgdict->checker) < 0) { return -1; } } - else if (PyErr_Occurred()) { +/* XXX later, maybe. + if (PyDict_GetItemRef((PyObject *)stgdict, &_Py _ID(_errcheck_), &ob) < 0) { return -1; } -/* XXX later, maybe. - ob = _PyDict_GetItemIdWithError((PyObject *)stgdict, &PyId__errcheck_); if (ob) { if (!PyCallable_Check(ob)) { PyErr_SetString(PyExc_TypeError, "_errcheck_ must be callable"); + Py_DECREF(ob); return -1; } - stgdict->errcheck = Py_NewRef(ob); - } - else if (PyErr_Occurred()) { - return -1; + stgdict->errcheck = ob; } */ return 0; @@ -3812,13 +3804,12 @@ _get_arg(int *pindex, PyObject *name, PyObject *defval, PyObject *inargs, PyObje return Py_NewRef(v); } if (kwds && name) { - v = PyDict_GetItemWithError(kwds, name); + if (PyDict_GetItemRef(kwds, name, &v) < 0) { + return NULL; + } if (v) { ++*pindex; - return Py_NewRef(v); - } - else if (PyErr_Occurred()) { - return NULL; + return v; } } if (defval) { @@ -4870,15 +4861,12 @@ PyCArrayType_from_ctype(PyObject *itemtype, Py_ssize_t length) return NULL; PyObject *result; - if (_PyDict_GetItemProxy(cache, key, &result) < 0) { - Py_DECREF(key); - return NULL; - } - if (result) { + if (_PyDict_GetItemProxy(cache, key, &result) != 0) { + // found or error Py_DECREF(key); return result; } - + // not found if (!PyType_Check(itemtype)) { PyErr_SetString(PyExc_TypeError, "Expected a type object"); diff --git a/Modules/_ctypes/callproc.c b/Modules/_ctypes/callproc.c index 2a0c26a..3b11cd7 100644 --- a/Modules/_ctypes/callproc.c +++ b/Modules/_ctypes/callproc.c @@ -168,16 +168,18 @@ _ctypes_get_errobj(int **pspace) if (error_object_name == NULL) return NULL; } - errobj = PyDict_GetItemWithError(dict, error_object_name); + if (PyDict_GetItemRef(dict, error_object_name, &errobj) < 0) { + return NULL; + } if (errobj) { if (!PyCapsule_IsValid(errobj, CTYPES_CAPSULE_NAME_PYMEM)) { PyErr_SetString(PyExc_RuntimeError, "ctypes.error_object is an invalid capsule"); + Py_DECREF(errobj); return NULL; } - Py_INCREF(errobj); } - else if (!PyErr_Occurred()) { + else { void *space = PyMem_Calloc(2, sizeof(int)); if (space == NULL) return NULL; @@ -192,9 +194,6 @@ _ctypes_get_errobj(int **pspace) return NULL; } } - else { - return NULL; - } *pspace = (int *)PyCapsule_GetPointer(errobj, CTYPES_CAPSULE_NAME_PYMEM); return errobj; } @@ -1922,13 +1921,11 @@ create_pointer_type(PyObject *module, PyObject *cls) PyTypeObject *typ; PyObject *key; - result = PyDict_GetItemWithError(_ctypes_ptrtype_cache, cls); - if (result) { - return Py_NewRef(result); - } - else if (PyErr_Occurred()) { - return NULL; + if (PyDict_GetItemRef(_ctypes_ptrtype_cache, cls, &result) != 0) { + // found or error + return result; } + // not found if (PyUnicode_CheckExact(cls)) { PyObject *name = PyUnicode_FromFormat("LP_%U", cls); result = PyObject_CallFunction((PyObject *)Py_TYPE(&PyCPointer_Type), @@ -1986,16 +1983,14 @@ create_pointer_inst(PyObject *module, PyObject *arg) PyObject *result; PyObject *typ; - typ = PyDict_GetItemWithError(_ctypes_ptrtype_cache, (PyObject *)Py_TYPE(arg)); - if (typ) { - return PyObject_CallOneArg(typ, arg); - } - else if (PyErr_Occurred()) { + if (PyDict_GetItemRef(_ctypes_ptrtype_cache, (PyObject *)Py_TYPE(arg), &typ) < 0) { return NULL; } - typ = create_pointer_type(NULL, (PyObject *)Py_TYPE(arg)); - if (typ == NULL) - return NULL; + if (typ == NULL) { + typ = create_pointer_type(NULL, (PyObject *)Py_TYPE(arg)); + if (typ == NULL) + return NULL; + } result = PyObject_CallOneArg(typ, arg); Py_DECREF(typ); return result; -- cgit v0.12