Skip to content

Commit

Permalink
Modernize DECREF (part 1). (#3203)
Browse files Browse the repository at this point in the history
This commit refactors a part of the NRN Python bindings to use `nanobind`
objects instead of `Py_DECREF`. The purpose is to simplify the DECREFing logic
on error paths; and reduce the risk of leaking when exceptions are thrown.

As part of the refactoring, if needed, the scope of certain variables might be
reduced or a given a new name. Additionally, NULL pointers are replaced with
`nullptr`.

This commit doesn't intentionally change reference counts.
  • Loading branch information
1uc authored Nov 15, 2024
1 parent 36084d9 commit 18b1c14
Showing 1 changed file with 21 additions and 27 deletions.
48 changes: 21 additions & 27 deletions src/nrnpython/nrnpy_nrn.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -404,13 +404,12 @@ static int NPySecObj_init(NPySecObj* self, PyObject* args, PyObject* kwds) {
if (cell) {
// include cellname in name so nrnpy_pysecname2sec_remove can determine

cell = PyObject_Str(cell);
if (cell == NULL) {
auto cell_str = nb::steal(PyObject_Str(cell));
if (!cell_str) {
Py_XDECREF(self->cell_weakref_);
return -1;
}
Py2NRNString str(cell);
Py_DECREF(cell);
Py2NRNString str(cell_str.ptr());
if (str.err()) {
str.set_pyerr(PyExc_TypeError, "cell name contains non ascii character");
return -1;
Expand Down Expand Up @@ -453,33 +452,29 @@ static int NPyAllSegOfSecIter_init_safe(NPyAllSegOfSecIter* self, PyObject* args
}

PyObject* NPySecObj_new(PyTypeObject* type, PyObject* args, PyObject* kwds) {
NPySecObj* self;
self = (NPySecObj*) type->tp_alloc(type, 0);
// printf("NPySecObj_new %p\n", self);
if (self != NULL) {
if (NPySecObj_init(self, args, kwds) != 0) {
Py_DECREF(self);
return NULL;
auto self = nb::steal(type->tp_alloc(type, 0));
// printf("NPySecObj_new %p\n", self.ptr());
if (self) {
if (NPySecObj_init((NPySecObj*) self.ptr(), args, kwds) != 0) {
return nullptr;
}
}
return (PyObject*) self;
return self.release().ptr();
}

PyObject* NPySecObj_new_safe(PyTypeObject* type, PyObject* args, PyObject* kwds) {
return nrn::convert_cxx_exceptions(NPySecObj_new, type, args, kwds);
}

PyObject* NPyAllSegOfSecIter_new(PyTypeObject* type, PyObject* args, PyObject* kwds) {
NPyAllSegOfSecIter* self;
self = (NPyAllSegOfSecIter*) type->tp_alloc(type, 0);
// printf("NPyAllSegOfSecIter_new %p\n", self);
if (self != NULL) {
if (NPyAllSegOfSecIter_init(self, args, kwds) != 0) {
Py_DECREF(self);
return NULL;
auto self = nb::steal(type->tp_alloc(type, 0));
// printf("NPyAllSegOfSecIter_new %p\n", self.ptr());
if (self) {
if (NPyAllSegOfSecIter_init((NPyAllSegOfSecIter*) self.ptr(), args, kwds) != 0) {
return nullptr;
}
}
return (PyObject*) self;
return self.release().ptr();
}

PyObject* NPyAllSegOfSecIter_new_safe(PyTypeObject* type, PyObject* args, PyObject* kwds) {
Expand Down Expand Up @@ -1860,17 +1855,16 @@ static PyObject* mech_of_segment_iter(NPySegObj* self) {
CHECK_SEC_INVALID(sec)
Node* nd = node_exact(sec, self->x_);
Prop* p = mech_of_segment_prop(nd->prop);
NPyMechOfSegIter* mi = PyObject_New(NPyMechOfSegIter, pmech_of_seg_iter_generic_type);
auto mi = nb::steal((PyObject*) PyObject_New(NPyMechOfSegIter, pmech_of_seg_iter_generic_type));
if (!mi) {
return NULL;
return nullptr;
}
NPyMechObj* m = new_pymechobj(self, p);
auto m = nb::steal((PyObject*) new_pymechobj(self, p));
if (!m) {
Py_XDECREF(mi);
return NULL;
return nullptr;
}
mi->pymech_ = m;
return (PyObject*) mi;
((NPyMechOfSegIter*) mi.ptr())->pymech_ = (NPyMechObj*) m.release().ptr();
return mi.release().ptr();
}

static PyObject* mech_of_segment_iter_safe(NPySegObj* self) {
Expand Down

0 comments on commit 18b1c14

Please sign in to comment.