From 614a9562477ba79d02b1c83ec04e5648ccd7db89 Mon Sep 17 00:00:00 2001 From: Ronald Oussoren Date: Sun, 14 Jul 2024 21:23:02 +0200 Subject: [PATCH] Phase out usage of PySequence_Fast Primary reason is that ownership of the result of PySequence_Fast_GET_ITEM is a borrowed reference that may be in a list. That's problematic in regular builds, and even more so in free threaded builds because the list can be mutated while using the item. Issue #608 --- .../Modules/_AppKit_nsbezierpath.m | 20 ++++++++-------- .../Modules/_AppKit_nsbitmap.m | 22 ++++++++---------- pyobjc-framework-Cocoa/Modules/testhelper.m | 4 +++- pyobjc-framework-CoreText/Modules/_manual.m | 23 +++++++++++-------- .../Modules/_coregraphics.m | 8 +++---- pyobjc-framework-Security/Modules/_Security.m | 18 +++++++-------- .../Modules/_SecurityInterface.m | 20 +++++++--------- 7 files changed, 57 insertions(+), 58 deletions(-) diff --git a/pyobjc-framework-Cocoa/Modules/_AppKit_nsbezierpath.m b/pyobjc-framework-Cocoa/Modules/_AppKit_nsbezierpath.m index 0cfd40ea08..e138e588ae 100644 --- a/pyobjc-framework-Cocoa/Modules/_AppKit_nsbezierpath.m +++ b/pyobjc-framework-Cocoa/Modules/_AppKit_nsbezierpath.m @@ -124,12 +124,12 @@ memset(points, 0, sizeof(points)); - seq = PySequence_Fast(pointList, "points is not a sequence"); + seq = PySequence_Tuple(pointList); if (seq == NULL) { return NULL; } - len = PySequence_Fast_GET_SIZE(seq); + len = PyTuple_GET_SIZE(seq); if (len > 3) { Py_DECREF(seq); PyErr_SetString(PyExc_ValueError, "Need at most 3 elements"); @@ -137,12 +137,13 @@ } for (i = 0; i < len; i++) { - int err = PyObjC_PythonToObjC(@encode(NSPoint), PySequence_Fast_GET_ITEM(seq, i), + int err = PyObjC_PythonToObjC(@encode(NSPoint), PyTuple_GET_ITEM(seq, i), points + i); if (err == -1) { return NULL; } } + Py_DECREF(seq); Py_BEGIN_ALLOW_THREADS @try { @@ -217,24 +218,23 @@ if (result == NULL) goto error; - seq = PySequence_Fast(result, "should return tuple of length 2"); + seq = PySequence_Tuple(result); Py_DECREF(result); if (seq == NULL) goto error; - if (PySequence_Fast_GET_SIZE(seq) != 2) { + if (PyTuple_GET_SIZE(seq) != 2) { PyErr_SetString(PyExc_ValueError, "should return tuple of length 2"); goto error; } - v = PySequence_Fast_GET_ITEM(seq, 0); + v = PyTuple_GET_ITEM(seq, 0); err = PyObjC_PythonToObjC(@encode(NSBezierPathElement), v, &element); if (err == -1) goto error; - v = PySequence_Fast(PySequence_Fast_GET_ITEM(seq, 1), - "result[1] should be a sequence"); + v = PySequence_Tuple(PyTuple_GET_ITEM(seq, 1)); if (v == NULL) goto error; @@ -262,14 +262,14 @@ goto error; } - if (PySequence_Fast_GET_SIZE(v) != pointCount) { + if (PyTuple_GET_SIZE(v) != pointCount) { PyErr_SetString(PyExc_ValueError, "wrong number of points"); Py_DECREF(v); goto error; } for (i = 0; i < pointCount; i++) { - err = PyObjC_PythonToObjC(@encode(NSPoint), PySequence_Fast_GET_ITEM(v, i), + err = PyObjC_PythonToObjC(@encode(NSPoint), PyTuple_GET_ITEM(v, i), points + i); if (err == -1) { Py_DECREF(v); diff --git a/pyobjc-framework-Cocoa/Modules/_AppKit_nsbitmap.m b/pyobjc-framework-Cocoa/Modules/_AppKit_nsbitmap.m index 63d6d5ba6b..07c4d2f5aa 100644 --- a/pyobjc-framework-Cocoa/Modules/_AppKit_nsbitmap.m +++ b/pyobjc-framework-Cocoa/Modules/_AppKit_nsbitmap.m @@ -92,23 +92,20 @@ } py_allPlanes = arguments[0]; if (py_allPlanes != Py_None) { - PyObject* fast_planes = - PySequence_Fast(py_allPlanes, "Expecting a 5 tuple or None"); + PyObject* fast_planes = PySequence_Tuple(py_allPlanes); if (fast_planes == NULL) { - /* XXX: Clearer error message */ PyErr_SetString(PyExc_TypeError, "First argument must be a 5 element Tuple or None."); return NULL; } - if (PySequence_Fast_GET_SIZE(fast_planes) != 5) { - /* XXX: Clearer error message */ + if (PyTuple_GET_SIZE(fast_planes) != 5) { PyErr_SetString(PyExc_TypeError, "First argument must be a 5 element Tuple or None."); Py_DECREF(fast_planes); return NULL; } for (i = 0; i < 5; i++) { - PyObject* tmp = PySequence_Fast_GET_ITEM(fast_planes, i); + PyObject* tmp = PyTuple_GET_ITEM(fast_planes, i); if (tmp == Py_None) { dataPlanes[i] = NULL; } else { @@ -116,10 +113,12 @@ if (r == 0) { dataPlanes[i] = planeBuffers[i].buf; } else { + Py_DECREF(fast_planes); goto error_cleanup; } } } + Py_DECREF(fast_planes); } if (PyObjC_PythonToObjC(@encode(int), arguments[1], &width) == -1) { @@ -213,23 +212,20 @@ } py_allPlanes = arguments[0]; if (py_allPlanes != Py_None) { - PyObject* fast_planes = - PySequence_Fast(py_allPlanes, "Expecting a 5 tuple or None"); + PyObject* fast_planes = PySequence_Tuple(py_allPlanes); if (fast_planes == NULL) { - /* XXX: Clearer error message */ PyErr_SetString(PyExc_TypeError, "First argument must be a 5 element Tuple or None."); return NULL; } - if (PySequence_Fast_GET_SIZE(fast_planes) != 5) { - /* XXX: Clearer error message */ + if (PyTuple_GET_SIZE(fast_planes) != 5) { PyErr_SetString(PyExc_TypeError, "First argument must be a 5 element Tuple or None."); Py_DECREF(fast_planes); return NULL; } for (i = 0; i < 5; i++) { - PyObject* tmp = PySequence_Fast_GET_ITEM(fast_planes, i); + PyObject* tmp = PyTuple_GET_ITEM(fast_planes, i); if (tmp == Py_None) { dataPlanes[i] = NULL; } else { @@ -237,10 +233,12 @@ if (r == 0) { dataPlanes[i] = planeBuffers[i].buf; } else { + Py_DECREF(fast_planes); goto error_cleanup; } } } + Py_DECREF(fast_planes); } if (PyObjC_PythonToObjC(@encode(int), arguments[1], &width) == -1) { diff --git a/pyobjc-framework-Cocoa/Modules/testhelper.m b/pyobjc-framework-Cocoa/Modules/testhelper.m index 2559780b7e..b5007045f9 100644 --- a/pyobjc-framework-Cocoa/Modules/testhelper.m +++ b/pyobjc-framework-Cocoa/Modules/testhelper.m @@ -242,12 +242,14 @@ + (NSString*)fetchObjectDescription:(NSObject*)value static int mod_exec_module(PyObject* m) { - if (PyObjC_ImportAPI(m) == -1) + if (PyObjC_ImportAPI(m) == -1) { return -1; + } if (PyModule_AddObject(m, "PyObjC_TestClass3", PyObjC_IdToPython([PyObjC_TestClass3 class])) < 0) { return -1; + } if (PyModule_AddObject(m, "PyObjC_TestClass4", PyObjC_IdToPython([PyObjC_TestClass4 class])) < 0) { return -1; diff --git a/pyobjc-framework-CoreText/Modules/_manual.m b/pyobjc-framework-CoreText/Modules/_manual.m index 2a1c32ddf8..9f46e011e9 100644 --- a/pyobjc-framework-CoreText/Modules/_manual.m +++ b/pyobjc-framework-CoreText/Modules/_manual.m @@ -149,12 +149,12 @@ return result; } - seq = PySequence_Fast(py_settings, "need sequence of settings"); + seq = PySequence_Tuple(py_settings); if (seq == NULL) { return NULL; } - if (PySequence_Fast_GET_SIZE(seq) < len) { + if (PyTuple_GET_SIZE(seq) < len) { PyErr_Format(PyExc_ValueError, "need sequence of at least %ld arguments", (long)len); Py_DECREF(seq); @@ -178,25 +178,25 @@ for (i = 0; i < len; i++) { CTParagraphStyleSetting* cur = settings + i; - PyObject* curPy = PySequence_Fast_GET_ITEM(seq, i); - PyObject* s = PySequence_Fast(curPy, "CTParagraphStyleItem"); + PyObject* curPy = PyTuple_GET_ITEM(seq, i); + PyObject* s = PySequence_Tuple(curPy); int r; if (s == NULL) { goto setup_error; } - if (PySequence_Fast_GET_SIZE(s) != 3) { + if (PyTuple_GET_SIZE(s) != 3) { PyErr_Format(PyExc_ValueError, "settings item has length %ld, not 3", - (long)PySequence_Fast_GET_SIZE(s)); + (long)PyTuple_GET_SIZE(s)); goto setup_error; } r = PyObjC_PythonToObjC(@encode(CTParagraphStyleSpecifier), - PySequence_Fast_GET_ITEM(s, 0), &cur->spec); + PyTuple_GET_ITEM(s, 0), &cur->spec); if (r == -1) { goto setup_error; } - r = PyObjC_PythonToObjC(@encode(size_t), PySequence_Fast_GET_ITEM(s, 1), + r = PyObjC_PythonToObjC(@encode(size_t), PyTuple_GET_ITEM(s, 1), &cur->valueSize); if (r == -1) { goto setup_error; @@ -212,11 +212,11 @@ } else { r = PyObjC_PythonToObjC(@encode(CFArrayRef), - PySequence_Fast_GET_ITEM(s, 2), &aref); + PyTuple_GET_ITEM(s, 2), &aref); cur->value = &aref; } } else { - r = PyObject_GetBuffer(PySequence_Fast_GET_ITEM(s, 2), views + i, + r = PyObject_GetBuffer(PyTuple_GET_ITEM(s, 2), views + i, PyBUF_CONTIG_RO); if (r != -1) { if ((size_t)views[i].len != cur->valueSize) { @@ -231,9 +231,12 @@ } } if (r == -1) { + Py_DECREF(s); goto setup_error; } + Py_DECREF(s); } + Py_DECREF(seq); CTParagraphStyleRef rv = NULL; diff --git a/pyobjc-framework-Quartz/Modules/_coregraphics.m b/pyobjc-framework-Quartz/Modules/_coregraphics.m index 55b7e259b1..cd3ba91c68 100644 --- a/pyobjc-framework-Quartz/Modules/_coregraphics.m +++ b/pyobjc-framework-Quartz/Modules/_coregraphics.m @@ -130,25 +130,25 @@ static CFArrayRef createWindowList(PyObject* items) { - PyObject* seq = PySequence_Fast(items, "list of windowIDs"); + PyObject* seq = PySequence_Tuple(items); if (seq == NULL) { return NULL; } CFMutableArrayRef array = - CFArrayCreateMutable(NULL, PySequence_Fast_GET_SIZE(seq), NULL); + CFArrayCreateMutable(NULL, PyTuple_GET_SIZE(seq), NULL); if (array == NULL) { Py_DECREF(seq); PyErr_SetString(PyExc_ValueError, "Cannot create CFArray"); return NULL; } - Py_ssize_t len = PySequence_Fast_GET_SIZE(seq); + Py_ssize_t len = PyTuple_GET_SIZE(seq); Py_ssize_t i; for (i = 0; i < len; i++) { CGWindowID windowID; - if (PyObjC_PythonToObjC(@encode(CGWindowID), PySequence_Fast_GET_ITEM(seq, i), + if (PyObjC_PythonToObjC(@encode(CGWindowID), PyTuple_GET_ITEM(seq, i), &windowID) == -1) { Py_DECREF(seq); diff --git a/pyobjc-framework-Security/Modules/_Security.m b/pyobjc-framework-Security/Modules/_Security.m index 8e1d92978f..5019f6f844 100644 --- a/pyobjc-framework-Security/Modules/_Security.m +++ b/pyobjc-framework-Security/Modules/_Security.m @@ -356,22 +356,22 @@ return 1; } else { - PyObject* seq = PySequence_Fast(value, "itemset must be a sequence or None"); + PyObject* seq = PySequence_Tuple(value); Py_ssize_t i; if (seq == NULL) { return 0; } - itemset->count = PySequence_Fast_GET_SIZE(seq); + itemset->count = PyTuple_GET_SIZE(seq); itemset->items = - PyMem_Malloc(sizeof(AuthorizationItem) * PySequence_Fast_GET_SIZE(seq)); + PyMem_Malloc(sizeof(AuthorizationItem) * PyTuple_GET_SIZE(seq)); if (itemset->items == NULL) { PyErr_NoMemory(); return 0; } - for (i = 0; i < PySequence_Fast_GET_SIZE(seq); i++) { + for (i = 0; i < PyTuple_GET_SIZE(seq); i++) { if (PyObjC_PythonToObjC("{_AuthorizationItem=^cL^vI}", - PySequence_Fast_GET_ITEM(seq, i), itemset->items + i) + PyTuple_GET_ITEM(seq, i), itemset->items + i) < 0) { PyMem_Free(itemset->items); return 0; @@ -720,12 +720,12 @@ pathToTool = PyBytes_AsString(py_pathToTool); - seq = PySequence_Fast(py_arguments, "arguments must be a sequence of byte strings"); + seq = PySequence_Tuple(py_arguments); if (seq == NULL) { return NULL; } - arguments = PyMem_Malloc(sizeof(char*) * PySequence_Fast_GET_SIZE(seq) + 1); + arguments = PyMem_Malloc(sizeof(char*) * PyTuple_GET_SIZE(seq) + 1); if (arguments == NULL) { PyErr_NoMemory(); return NULL; @@ -736,8 +736,8 @@ return NULL; } - for (i = 0; i < PySequence_Fast_GET_SIZE(seq); i++) { - PyObject* t = PySequence_Fast_GET_ITEM(seq, i); + for (i = 0; i < PyTuple_GET_SIZE(seq); i++) { + PyObject* t = PyTuple_GET_ITEM(seq, i); if (!PyBytes_Check(t)) { PyErr_SetString(PyExc_ValueError, diff --git a/pyobjc-framework-SecurityInterface/Modules/_SecurityInterface.m b/pyobjc-framework-SecurityInterface/Modules/_SecurityInterface.m index 82cd3e32ed..d488311c15 100644 --- a/pyobjc-framework-SecurityInterface/Modules/_SecurityInterface.m +++ b/pyobjc-framework-SecurityInterface/Modules/_SecurityInterface.m @@ -2,13 +2,6 @@ #include "Python.h" #include "pyobjc-api.h" -#undef PySequence_Fast_GET_ITEM -#define PySequence_Fast_GET_ITEM(o, i) \ - (PyList_Check(o) ? PyList_GetItem(o, i) : PyTuple_GetItem(o, i)) - -#undef PySequence_Fast_GET_SIZE -#define PySequence_Fast_GET_SIZE(o) (PyList_Check(o) ? PyList_Size(o) : PyTuple_Size(o)) - #import #import @@ -21,27 +14,30 @@ return 1; } else { - PyObject* seq = PySequence_Fast(value, "itemset must be a sequence or None"); + PyObject* seq = PySequence_Tuple(value); Py_ssize_t i; if (seq == NULL) { return 0; } - itemset->count = PySequence_Fast_GET_SIZE(seq); + itemset->count = PyTuple_GET_SIZE(seq); itemset->items = - PyMem_Malloc(sizeof(AuthorizationItem) * PySequence_Fast_GET_SIZE(seq)); + PyMem_Malloc(sizeof(AuthorizationItem) * PyTuple_GET_SIZE(seq)); if (itemset->items == NULL) { PyErr_NoMemory(); + Py_DECREF(seq); return 0; } - for (i = 0; i < PySequence_Fast_GET_SIZE(seq); i++) { + for (i = 0; i < PyTuple_GET_SIZE(seq); i++) { if (PyObjC_PythonToObjC("{_AuthorizationItem=^cL^vI}", - PySequence_Fast_GET_ITEM(seq, i), itemset->items + i) + PyTuple_GET_ITEM(seq, i), itemset->items + i) < 0) { PyMem_Free(itemset->items); + Py_DECREF(seq); return 0; } } + Py_DECREF(seq); } return 1; }