Skip to content

Commit

Permalink
First step in migrating to PyDict_GetItemRef
Browse files Browse the repository at this point in the history 
First steps in migrating to PyDict_GetItemRef for fetching
items from a dict because the older APIs return a borrowed
reference.

Also stop using "PyDict.*String" APIs, explicitly construct
a string object where needed and use *PyObjCNM* constants
when using constant strings.

The changeset also marks unwanted APIs as unavailable in
"python-api-used.h" to ensure that there will be compile errors
when building using the static analyser.

Finally disable optimization for now to easy debugging while
reworking pyobjc-core (will be renabled later).

Issue #608
  • Loading branch information
@ronaldoussoren
ronaldoussoren committed Jul 23, 2024
1 parent a6b502a commit f3e13c7
Show file tree
Hide file tree
Showing 20 changed files with 388 additions and 207 deletions.
22 changes: 14 additions & 8 deletions pyobjc-core/Modules/objc/OC_PythonDictionary.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,23 +203,29 @@ - (id _Nullable)objectForKey:key
}

if (likely(PyDict_CheckExact(value))) {
v = PyDict_GetItemWithError(value, k);
if (v == NULL && PyErr_Occurred()) { // LCOV_BR_EXCL_LINE
int r = PyDict_GetItemRef(value, k, &v);
switch (r) {
case -1:
PyObjC_GIL_FORWARD_EXC(); // LCOV_EXCL_LINE
case 0:
PyObjC_GIL_RETURN(nil);
case 1:
break;
}
Py_XINCREF(v);

} else {
v = PyObject_GetItem(value, k);
if (v == nil) {
if (PyErr_ExceptionMatches(PyExc_KeyError)) {
PyErr_Clear();
PyObjC_GIL_RETURN(nil);
}
PyObjC_GIL_FORWARD_EXC();
}
}

Py_DECREF(k);

if (unlikely(v == NULL)) {
PyErr_Clear();
PyObjC_GIL_RETURN(nil);
}

if (v == Py_None) {
result = [NSNull null]; /* XXX: NSNull_null */

Expand Down
111 changes: 64 additions & 47 deletions pyobjc-core/Modules/objc/corefoundation.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,27 +47,34 @@

PyObject* cfid;
PyTypeObject* tp;
int r;

cfid = PyLong_FromLong(CFGetTypeID((CFTypeRef)value));
if (cfid == NULL) { // LCOV_BR_EXCL_LINE
return NULL; // LCOV_EXCL_LINE
}
tp = (PyTypeObject*)PyDict_GetItemWithError(gTypeid2class, cfid);
r = PyDict_GetItemRef(gTypeid2class, cfid, (PyObject**)&tp);
Py_DECREF(cfid);
if (tp == NULL) {
switch (r) {
case -1:
return NULL;
}

rval = tp->tp_alloc(tp, 0);
if (rval == NULL) { // LCOV_BR_EXCL_LINE
return NULL; // LCOV_EXCL_LINE
}
case 0:
return NULL;

((PyObjCObject*)rval)->objc_object = value;
((PyObjCObject*)rval)->flags = PyObjCObject_kDEFAULT | PyObjCObject_kCFOBJECT;
CFRetain(value);
default:
rval = tp->tp_alloc(tp, 0);
Py_DECREF(tp);
if (rval == NULL) { // LCOV_BR_EXCL_LINE
return NULL; // LCOV_EXCL_LINE
}

return rval;
((PyObjCObject*)rval)->objc_object = value;
((PyObjCObject*)rval)->flags = PyObjCObject_kDEFAULT | PyObjCObject_kCFOBJECT;
CFRetain(value);

return rval;
}
}

#ifdef PyObjC_ENABLE_CFTYPE_CATEGORY
Expand Down Expand Up @@ -108,6 +115,7 @@ SEL _sel __attribute__((__unused__)))
PyObject* result;
PyObject* bases;
PyObjCClassObject* info;
int r;

if (encoding[0] != _C_ID) {
if (PyObjCPointerWrapper_RegisterID(name, encoding) == -1) { // LCOV_BR_EXCL_LINE
Expand Down Expand Up @@ -138,22 +146,20 @@ SEL _sel __attribute__((__unused__)))
return NULL; // LCOV_EXCL_LINE
}

result = PyDict_GetItemWithError(gTypeid2class, cf);
if (result == NULL && PyErr_Occurred()) { // LCOV_BR_EXCL_LINE
// LCOV_EXCL_START
r = PyDict_GetItemRef(gTypeid2class, cf, &result);
switch (r) {
case -1:
Py_DECREF(cf);
return NULL;
// LCOV_EXCL_STOP
}
if (result != NULL) {
case 1:
/* This type is the same as an already registered type,
* return that type
*/
Py_DECREF(cf);
Py_INCREF(result);
return result;
}

}
/* case 0: */
dict = PyDict_New();
if (dict == NULL) { // LCOV_BR_EXCL_LINE
// LCOV_EXCL_START
Expand All @@ -163,8 +169,8 @@ SEL _sel __attribute__((__unused__)))
}

/* XXX: This leaks the new tuple */
if (PyDict_SetItemString( // LCOV_BR_EXCL_LINE
dict, "__slots__", PyTuple_New(0))
if (PyDict_SetItem( // LCOV_BR_EXCL_LINE
dict, PyObjCNM___slots__, PyTuple_New(0))
== -1) {
// LCOV_EXCL_START
Py_DECREF(dict);
Expand Down Expand Up @@ -322,24 +328,33 @@ SEL _sel __attribute__((__unused__)))
*/
PyObject* _Nullable PyObjCCF_NewSpecialFromTypeEncoding(char* typestr, void* datum)
{
PyObject* v = PyDict_GetItemStringWithError(PyObjC_TypeStr2CFTypeID, typestr);
PyObject* v;
PyObject* typestr_obj = PyUnicode_FromString(typestr);
if (typestr_obj == NULL) {
return NULL;
}
int r = PyDict_GetItemRef(PyObjC_TypeStr2CFTypeID, typestr_obj, &v);
Py_DECREF(typestr_obj);

CFTypeID typeid;

if (v == NULL) {
if (PyErr_Occurred()) { // LCOV_BR_EXCL_LINE
return NULL; // LCOV_EXCL_LINE
}
switch (r) {
case -1:
return NULL;
case 0:
PyErr_Format(PyExc_ValueError,
"Don't know CF type for typestr '%s', cannot create special wrapper",
typestr);
return NULL;
}
default:
if (depythonify_c_value(@encode(CFTypeID), v, &typeid) < 0) { // LCOV_BR_EXCL_LINE
Py_DECREF(v);
return NULL; // LCOV_EXCL_LINE
}
Py_DECREF(v);

if (depythonify_c_value(@encode(CFTypeID), v, &typeid) < 0) { // LCOV_BR_EXCL_LINE
return NULL; // LCOV_EXCL_LINE
return PyObjCCF_NewSpecialFromTypeID(typeid, datum);
}

return PyObjCCF_NewSpecialFromTypeID(typeid, datum);
}

/*
Expand All @@ -352,33 +367,35 @@ SEL _sel __attribute__((__unused__)))
PyObjCCF_NewSpecialFromTypeID(CFTypeID typeid, void* datum)
{
PyObject* rval = NULL;
int r;

PyObjC_Assert(gTypeid2class != NULL, NULL);

PyObject* cfid;
PyTypeObject* tp;

cfid = PyLong_FromLong(typeid);
tp = (PyTypeObject*)PyDict_GetItemWithError(gTypeid2class, cfid);
r = PyDict_GetItemRef(gTypeid2class, cfid, (PyObject**)&tp);
Py_DECREF(cfid);
if (tp == NULL) {
if (!PyErr_Occurred()) { // LCOV_BR_EXCL_LINE
return (PyObject*)PyObjCObject_New(
datum, PyObjCObject_kMAGIC_COOKIE | PyObjCObject_kSHOULD_NOT_RELEASE, NO);
switch (r) {
case -1:
return NULL;
case 0:
return (PyObject*)PyObjCObject_New(
datum, PyObjCObject_kMAGIC_COOKIE | PyObjCObject_kSHOULD_NOT_RELEASE, NO);
default:
rval = tp->tp_alloc(tp, 0);
Py_DECREF(tp);
if (rval == NULL) { // LCOV_BR_EXCL_LINE
return NULL; // LCOV_EXCL_LINE
}
return NULL; // LCOV_EXCL_LINE
}

rval = tp->tp_alloc(tp, 0);
if (rval == NULL) { // LCOV_BR_EXCL_LINE
return NULL; // LCOV_EXCL_LINE
((PyObjCObject*)rval)->objc_object = datum;
((PyObjCObject*)rval)->flags = PyObjCObject_kDEFAULT
| PyObjCObject_kSHOULD_NOT_RELEASE
| PyObjCObject_kMAGIC_COOKIE;
return rval;
}

((PyObjCObject*)rval)->objc_object = datum;
((PyObjCObject*)rval)->flags = PyObjCObject_kDEFAULT
| PyObjCObject_kSHOULD_NOT_RELEASE
| PyObjCObject_kMAGIC_COOKIE;
return rval;
}

NS_ASSUME_NONNULL_END
4 changes: 2 additions & 2 deletions pyobjc-core/Modules/objc/function.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@
return NULL; // LCOV_EXCL_LINE
}
if (self->doc) {
if (PyDict_SetItemString( // LCOV_BR_EXCL_LINE
result, "__doc__", self->doc)
if (PyDict_SetItem( // LCOV_BR_EXCL_LINE
result, PyObjCNM___doc__, self->doc)
== -1) {
Py_DECREF(result); // LCOV_EXCL_LINE
return NULL; // LCOV_EXCL_LINE
Expand Down
3 changes: 1 addition & 2 deletions pyobjc-core/Modules/objc/helpers-authorization.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,12 @@
{
const struct auth_item_set* value = (const struct auth_item_set*)_value;
PyObject* result;
const char* oc_typestr;
Py_ssize_t pack;
int have_tuple = 0;
PyObject* t;
int r;

result = PyObjC_CreateRegisteredStruct("{_AuthorizationItem=^cL^vI}", 27, &oc_typestr,
result = PyObjC_CreateRegisteredStruct("{_AuthorizationItem=^cL^vI}", 27, NULL,
&pack);
if (result == NULL) {
have_tuple = 1;
Expand Down
2 changes: 1 addition & 1 deletion pyobjc-core/Modules/objc/helpers-foundation-nsdecimal.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1106,7 +1106,7 @@ static int decimal_init(PyObject* self, PyObject* _Nullable args,

Class classNSDecimalNumberPlaceholder =
objc_lookUpClass("NSDecimalNumberPlaceholder");
if (classNSDecimalNumberPlaceholder != nil) { // LCOV_BR_EXCL_LINE
if (classNSDecimalNumberPlaceholder != Nil) { // LCOV_BR_EXCL_LINE
if (PyObjC_RegisterMethodMapping(classNSDecimalNumberPlaceholder,
@selector(initWithDecimal:),
call_NSDecimalNumber_initWithDecimal_,
Expand Down
2 changes: 2 additions & 0 deletions pyobjc-core/Modules/objc/libffi_support.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ struct byref_attr {
} \
}

extern int PyObjCFFI_Setup(PyObject* m);

typedef void (*PyObjCFFI_ClosureFunc)(ffi_cif*, void*, void* _Nullable* _Nonnull, void*);
typedef void (*PyObjC_callback_function)(void);
typedef void (*PyObjCBlockFunction)(void*, ...);
Expand Down
75 changes: 47 additions & 28 deletions pyobjc-core/Modules/objc/libffi_support.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,23 @@
#error "Need FFI_CLOSURES!"
#endif


static PyObject* array_types = NULL;
static PyObject* struct_types = NULL;

int PyObjCFFI_Setup(PyObject* m __attribute__((__unused__)))
{
array_types = PyDict_New();
if (array_types == NULL) {// LCOV_BR_EXCL_LINE
return -1; // LCOV_EXCL_LINE
}
struct_types = PyDict_New();
if (struct_types == NULL) {
return -1;
}
return 0;
}

#ifdef PyObjC_DEBUG
/*
* describe_ffitype and describe_cif are useful during debugging,
Expand Down Expand Up @@ -186,26 +203,31 @@

static ffi_type* _Nullable array_to_ffi_type(const char* argtype)
{
static PyObject* array_types = NULL;

PyObject* v;
ffi_type* type;
Py_ssize_t field_count;
Py_ssize_t i;
const char* key = argtype;

if (array_types == NULL) {
array_types = PyDict_New();
if (array_types == NULL) // LCOV_BR_EXCL_LINE
return NULL; // LCOV_EXCL_LINE
}
PyObjC_Assert(array_types != NULL, NULL);

v = PyDict_GetItemStringWithError(array_types, (char*)argtype);
if (v == NULL && PyErr_Occurred()) { // LCOV_BR_EXCL_LINE
return NULL; // LCOV_EXCL_LINE
PyObject* typestr = PyUnicode_FromString(argtype);
if (typestr == NULL) {
return NULL;
}
if (v != NULL) {
return (ffi_type*)PyCapsule_GetPointer(v, "objc.__ffi_type__");
switch (PyDict_GetItemRef(array_types, typestr, &v)) {
case -1:
Py_DECREF(typestr);
return NULL;
case 1:
/* XXX: This effectively returns a borrowed reference */
Py_DECREF(typestr);
ffi_type* result = (ffi_type*)PyCapsule_GetPointer(v, "objc.__ffi_type__");
Py_DECREF(v);
return result;
default:
Py_DECREF(typestr);
}

/* We don't have a type description yet, dynamically
Expand Down Expand Up @@ -276,30 +298,27 @@

static ffi_type* _Nullable struct_to_ffi_type(const char* argtype)
{
/*
* XXX: Move to a central place.
*/
static PyObject* struct_types = NULL;

PyObject* v;
ffi_type* type;
Py_ssize_t field_count;
const char* curtype;

if (struct_types == NULL) { /* LCOV_BR_EXCL_LINE */
// LCOV_EXCL_START
struct_types = PyDict_New();
if (struct_types == NULL)
return NULL;
// LCOV_EXCL_STOP
PyObject* typestr = PyUnicode_FromString(argtype);
if (typestr == NULL) {
return NULL;
}

v = PyDict_GetItemStringWithError(struct_types, (char*)argtype);
if (v == NULL && PyErr_Occurred()) { // LCOV_BR_EXCL_LINE
switch (PyDict_GetItemRef(struct_types, typestr, &v)) {
case -1:
Py_DECREF(typestr);
return NULL; // LCOV_EXCL_LINE
}
if (v != NULL) {
return (ffi_type*)PyCapsule_GetPointer(v, "objc.__ffi_type__");
case 1:
/* XXX: This effectively returns a borrowed reference */
Py_DECREF(typestr);
ffi_type* result = (ffi_type*)PyCapsule_GetPointer(v, "objc.__ffi_type__");
Py_DECREF(v);
return result;
default:
Py_DECREF(typestr);
}

/* We don't have a type description yet, dynamically
Expand Down
Loading

0 comments on commit f3e13c7

Please sign in to comment.