Skip to content

Commit

Permalink
Phase out usage of PySequence_Fast
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@ronaldoussoren
ronaldoussoren committed Jul 14, 2024
1 parent 3ca2d58 commit 614a956
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 58 deletions.
20 changes: 10 additions & 10 deletions pyobjc-framework-Cocoa/Modules/_AppKit_nsbezierpath.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,25 +124,26 @@

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");
return NULL;
}

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 {
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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);
Expand Down
22 changes: 10 additions & 12 deletions pyobjc-framework-Cocoa/Modules/_AppKit_nsbitmap.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,34 +92,33 @@
}
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 {
int r = PyObject_GetBuffer(tmp, planeBuffers + i, PyBUF_SIMPLE);
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) {
Expand Down Expand Up @@ -213,34 +212,33 @@
}
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 {
int r = PyObject_GetBuffer(tmp, planeBuffers + i, PyBUF_SIMPLE);
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) {
Expand Down
4 changes: 3 additions & 1 deletion pyobjc-framework-Cocoa/Modules/testhelper.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
23 changes: 13 additions & 10 deletions pyobjc-framework-CoreText/Modules/_manual.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
Expand All @@ -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) {
Expand All @@ -231,9 +231,12 @@
}
}
if (r == -1) {
Py_DECREF(s);
goto setup_error;
}
Py_DECREF(s);
}
Py_DECREF(seq);

CTParagraphStyleRef rv = NULL;

Expand Down
8 changes: 4 additions & 4 deletions pyobjc-framework-Quartz/Modules/_coregraphics.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
18 changes: 9 additions & 9 deletions pyobjc-framework-Security/Modules/_Security.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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,
Expand Down
20 changes: 8 additions & 12 deletions pyobjc-framework-SecurityInterface/Modules/_SecurityInterface.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 <Foundation/Foundation.h>
#import <SecurityInterface/SFAuthorizationView.h>

Expand All @@ -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;
}
Expand Down

0 comments on commit 614a956

Please sign in to comment.