Skip to content

Commit

Permalink
Refactor Py2NRNString.
Browse files Browse the repository at this point in the history
  • Loading branch information
@1uc
1uc committed Nov 26, 2024
1 parent bc6c2f9 commit 9d6872b
Show file tree
Hide file tree
Showing 7 changed files with 85 additions and 154 deletions.
4 changes: 3 additions & 1 deletion src/neuron/unique_cstr.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
#include <utility>
#include <ostream>

#include "nrnpython/nrn_export.hpp"

namespace neuron {

/** A RAII wrapper for C-style strings.
Expand All @@ -13,7 +15,7 @@ namespace neuron {
* ownership, this is achieved using `.release()`, which returns the contained C-string and makes
* this object invalid.
*/
class unique_cstr {
class NRN_EXPORT unique_cstr {
public:
unique_cstr(const unique_cstr&) = delete;
unique_cstr(unique_cstr&& other) {
Expand Down
5 changes: 3 additions & 2 deletions src/nrnpython/inithoc.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,9 @@ static int add_neuron_options() {
PySys_WriteStdout("A neuron_options key:value is not a string:string or string:None\n");
continue;
}
Py2NRNString skey(key);
Py2NRNString sval(value);

auto skey = Py2NRNString::as_ascii(key);
auto sval = Py2NRNString::as_ascii(value);
if (strcmp(skey.c_str(), "-print-options") == 0) {
rval = 1;
continue;
Expand Down
47 changes: 23 additions & 24 deletions src/nrnpython/nrnpy_hoc.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -420,21 +420,19 @@ int hocobj_pushargs(PyObject* args, std::vector<neuron::unique_cstr>& s2free) {
hoc_pushx(static_cast<double>(pn));
} else if (is_python_string(po)) {
char** ts = hoc_temp_charptr();
Py2NRNString str(po, /* disable_release */ true);
if (str.err()) {
auto str = Py2NRNString::as_ascii(po);
if (!str.is_valid()) {
// Since Python error has been set, need to clear, or hoc_execerror
// printing with Printf will generate a
// Exception ignored on calling ctypes callback function.
// So get the message, clear, and make the message
// part of the execerror.
auto err = neuron::unique_cstr(str.get_pyerr());
*ts = err.c_str();
s2free.push_back(std::move(err));
auto err = Py2NRNString::get_pyerr();
hoc_execerr_ext("python string arg cannot decode into c_str. Pyerr message: %s",
*ts);
err.c_str());
}
*ts = str.c_str();
s2free.push_back(neuron::unique_cstr(*ts));
s2free.push_back(std::move(str));
hoc_pushstr(ts);
} else if (PyObject_TypeCheck(po, hocobject_type)) {
// The PyObject_TypeCheck above used to be PyObject_IsInstance. The
Expand Down Expand Up @@ -1034,10 +1032,10 @@ static PyObject* hocobj_getattr(PyObject* subself, PyObject* pyname) {

PyObject* result = NULL;
int isptr = 0;
Py2NRNString name(pyname);
auto name = Py2NRNString::as_ascii(pyname);
char* n = name.c_str();
if (!n) {
name.set_pyerr(PyExc_TypeError, "attribute name must be a string");
Py2NRNString::set_pyerr(PyExc_TypeError, "attribute name must be a string");
return NULL;
}

Expand Down Expand Up @@ -1425,10 +1423,10 @@ static int hocobj_setattro(PyObject* subself, PyObject* pyname, PyObject* value)
if (self->type_ == PyHoc::HocObject && !self->ho_) {
return 1;
}
Py2NRNString name(pyname);
auto name = Py2NRNString::as_ascii(pyname);
char* n = name.c_str();
if (!n) {
name.set_pyerr(PyExc_TypeError, "attribute name must be a string");
Py2NRNString::set_pyerr(PyExc_TypeError, "attribute name must be a string");
return -1;
}
// printf("hocobj_setattro %s\n", n);
Expand Down Expand Up @@ -2228,9 +2226,10 @@ static PyObject* mkref(PyObject* self, PyObject* args) {
} else if (is_python_string(pa)) {
result->type_ = PyHoc::HocRefStr;
result->u.s_ = 0;
Py2NRNString str(pa);
if (str.err()) {
str.set_pyerr(PyExc_TypeError, "string arg must have only ascii characters");
auto str = Py2NRNString::as_ascii(pa);
if (!str.is_valid()) {
Py2NRNString::set_pyerr(PyExc_TypeError,
"string arg must have only ascii characters");
return NULL;
}
char* cpa = str.c_str();
Expand Down Expand Up @@ -2283,10 +2282,11 @@ static PyObject* setpointer(PyObject* self, PyObject* args) {
if (hpp->type_ != PyHoc::HocObject) {
goto done;
}
Py2NRNString str(name);
auto str = Py2NRNString::as_ascii(name);
char* n = str.c_str();
if (str.err()) {
str.set_pyerr(PyExc_TypeError, "POINTER name can contain only ascii characters");
if (!str.is_valid()) {
Py2NRNString::set_pyerr(PyExc_TypeError,
"POINTER name can contain only ascii characters");
return NULL;
}
Symbol* sym = getsym(n, hpp->ho_, 0);
Expand Down Expand Up @@ -2470,7 +2470,7 @@ static char* double_array_interface(PyObject* po, long& stride) {
PyObject* psize;
if (PyObject_HasAttrString(po, "__array_interface__")) {
auto ai = nb::steal(PyObject_GetAttrString(po, "__array_interface__"));
Py2NRNString typestr(PyDict_GetItemString(ai.ptr(), "typestr"));
auto typestr = Py2NRNString::as_ascii(PyDict_GetItemString(ai.ptr(), "typestr"));
if (strcmp(typestr.c_str(), array_interface_typestr) == 0) {
data = PyLong_AsVoidPtr(PyTuple_GetItem(PyDict_GetItemString(ai.ptr(), "data"), 0));
// printf("double_array_interface idata = %ld\n", idata);
Expand Down Expand Up @@ -2736,8 +2736,7 @@ static char** gui_helper_3_str_(const char* name, Object* obj, int handle_strptr
if (gui_callback) {
auto po = nb::steal(gui_helper_3_helper_(name, obj, handle_strptr));
char** ts = hoc_temp_charptr();
Py2NRNString str(po.ptr(), true);
*ts = str.c_str();
*ts = Py2NRNString::as_ascii(po.ptr()).release();
// TODO: is there a memory leak here? do I need to: s2free.push_back(*ts);
return ts;
}
Expand Down Expand Up @@ -3166,7 +3165,7 @@ char get_endian_character() {
return 0;
}

Py2NRNString byteorder(pbo);
auto byteorder = Py2NRNString::as_ascii(pbo);
if (byteorder.c_str() == NULL) {
return 0;
}
Expand Down Expand Up @@ -3271,9 +3270,9 @@ static char* nrncore_arg(double tstop) {
if (ts) {
auto arg = nb::steal(PyObject_CallObject(callable.ptr(), ts.ptr()));
if (arg) {
Py2NRNString str(arg.ptr());
if (str.err()) {
str.set_pyerr(
auto str = Py2NRNString::as_ascii(arg.ptr());
if (!str.is_valid()) {
Py2NRNString::set_pyerr(
PyExc_TypeError,
"neuron.coreneuron.nrncore_arg() must return an ascii string");
return nullptr;
Expand Down
43 changes: 22 additions & 21 deletions src/nrnpython/nrnpy_nrn.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -409,9 +409,10 @@ static int NPySecObj_init(NPySecObj* self, PyObject* args, PyObject* kwds) {
Py_XDECREF(self->cell_weakref_);
return -1;
}
Py2NRNString str(cell_str.ptr());
if (str.err()) {
str.set_pyerr(PyExc_TypeError, "cell name contains non ascii character");
auto str = Py2NRNString::as_ascii(cell_str.ptr());
if (!str.is_valid()) {
Py2NRNString::set_pyerr(PyExc_TypeError,
"cell name contains non ascii character");
Py_XDECREF(self->cell_weakref_);
return -1;
}
Expand Down Expand Up @@ -1947,10 +1948,10 @@ static PyObject* section_getattro(NPySecObj* self, PyObject* pyname) {
CHECK_SEC_INVALID(sec);
PyObject* rv;
auto _pyname_tracker = nb::borrow(pyname); // keep refcount+1 during use
Py2NRNString name(pyname);
auto name = Py2NRNString::as_ascii(pyname);
char* n = name.c_str();
if (name.err()) {
name.set_pyerr(PyExc_TypeError, "attribute name must be a string");
if (!name.is_valid()) {
Py2NRNString::set_pyerr(PyExc_TypeError, "attribute name must be a string");
return nullptr;
}
// printf("section_getattr %s\n", n);
Expand Down Expand Up @@ -2007,10 +2008,10 @@ static int section_setattro(NPySecObj* self, PyObject* pyname, PyObject* value)
PyObject* rv;
int err = 0;
auto _pyname_tracker = nb::borrow(pyname); // keep refcount+1 during use
Py2NRNString name(pyname);
auto name = Py2NRNString::as_ascii(pyname);
char* n = name.c_str();
if (name.err()) {
name.set_pyerr(PyExc_TypeError, "attribute name must be a string");
if (!name.is_valid()) {
Py2NRNString::set_pyerr(PyExc_TypeError, "attribute name must be a string");
return -1;
}
// printf("section_setattro %s\n", n);
Expand Down Expand Up @@ -2160,10 +2161,10 @@ static PyObject* segment_getattro(NPySegObj* self, PyObject* pyname) {

Symbol* sym;
auto _pyname_tracker = nb::borrow(pyname); // keep refcount+1 during use
Py2NRNString name(pyname);
auto name = Py2NRNString::as_ascii(pyname);
char* n = name.c_str();
if (name.err()) {
name.set_pyerr(PyExc_TypeError, "attribute name must be a string");
if (!name.is_valid()) {
Py2NRNString::set_pyerr(PyExc_TypeError, "attribute name must be a string");
return nullptr;
}
// printf("segment_getattr %s\n", n);
Expand Down Expand Up @@ -2302,10 +2303,10 @@ static int segment_setattro(NPySegObj* self, PyObject* pyname, PyObject* value)
Symbol* sym;
int err = 0;
auto _pyname_tracker = nb::borrow(pyname); // keep refcount+1 during use
Py2NRNString name(pyname);
auto name = Py2NRNString::as_ascii(pyname);
char* n = name.c_str();
if (name.err()) {
name.set_pyerr(PyExc_TypeError, "attribute name must be a string");
if (!name.is_valid()) {
Py2NRNString::set_pyerr(PyExc_TypeError, "attribute name must be a string");
return -1;
}
// printf("segment_setattro %s\n", n);
Expand Down Expand Up @@ -2451,10 +2452,10 @@ static PyObject* mech_getattro(NPyMechObj* self, PyObject* pyname) {
CHECK_SEC_INVALID(sec)
CHECK_PROP_INVALID(self->prop_id_);
auto _pyname_tracker = nb::borrow(pyname); // keep refcount+1 during use
Py2NRNString name(pyname);
auto name = Py2NRNString::as_ascii(pyname);
char* n = name.c_str();
if (!n) {
name.set_pyerr(PyExc_TypeError, "attribute name must be a string");
Py2NRNString::set_pyerr(PyExc_TypeError, "attribute name must be a string");
return nullptr;
}
// printf("mech_getattro %s\n", n);
Expand Down Expand Up @@ -2545,10 +2546,10 @@ static int mech_setattro(NPyMechObj* self, PyObject* pyname, PyObject* value) {

int err = 0;
auto _pyname_tracker = nb::borrow(pyname); // keep refcount+1 during use
Py2NRNString name(pyname);
auto name = Py2NRNString::as_ascii(pyname);
char* n = name.c_str();
if (name.err()) {
name.set_pyerr(PyExc_TypeError, "attribute name must be a string");
if (!name.is_valid()) {
Py2NRNString::set_pyerr(PyExc_TypeError, "attribute name must be a string");
return -1;
}
// printf("mech_setattro %s\n", n);
Expand Down Expand Up @@ -2598,7 +2599,7 @@ neuron::container::generic_data_handle* nrnpy_setpointer_helper(PyObject* pyname
NPyMechObj* m = (NPyMechObj*) mech;
Symbol* msym = memb_func[m->type_].sym;
char buf[200];
Py2NRNString name(pyname);
auto name = Py2NRNString::as_ascii(pyname);
char* n = name.c_str();
if (!n) {
return nullptr;
Expand Down
11 changes: 5 additions & 6 deletions src/nrnpython/nrnpy_p2h.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,9 +172,8 @@ static void py2n_component(Object* ob, Symbol* sym, int nindex, int isfunc) {
for (i = 0; i < nindex; ++i) {
nb::object arg = nb::steal(nrnpy_hoc_pop("isfunc py2n_component"));
if (!arg) {
PyErr2NRNString e;
e.get_pyerr();
hoc_execerr_ext("arg %d error: %s", i, e.c_str());
auto err = Py2NRNString::get_pyerr();
hoc_execerr_ext("arg %d error: %s", i, err.c_str());
}
args.append(arg);
}
Expand Down Expand Up @@ -231,8 +230,8 @@ static void py2n_component(Object* ob, Symbol* sym, int nindex, int isfunc) {
hoc_pushx(d);
} else if (is_python_string(result.ptr())) {
char** ts = hoc_temp_charptr();
Py2NRNString str(result.ptr(), true);
*ts = str.c_str();
// TODO double check that this doesn't leak.
*ts = Py2NRNString::as_ascii(result.ptr()).release();
hoc_pop_defer();
hoc_pushstr(ts);
} else {
Expand Down Expand Up @@ -363,7 +362,7 @@ static int hoccommand_exec_strret(Object* ho, char* buf, int size) {
nb::object r = hoccommand_exec_help(ho);
if (r.is_valid()) {
nb::str pn(r);
Py2NRNString str(pn.ptr());
auto str = Py2NRNString::as_ascii(pn.ptr());
strncpy(buf, str.c_str(), size);
buf[size - 1] = '\0';
} else {
Expand Down
68 changes: 21 additions & 47 deletions src/nrnpython/nrnpy_utils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,8 @@ inline std::tuple<nb::object, nb::object, nb::object> fetch_pyerr() {
return std::make_tuple(nb::steal(ptype), nb::steal(pvalue), nb::steal(ptraceback));
}


Py2NRNString::Py2NRNString(PyObject* python_string, bool disable_release) {
disable_release_ = disable_release;
str_ = NULL;
neuron::unique_cstr Py2NRNString::as_ascii(PyObject* python_string) {
char* str_ = nullptr;
if (PyUnicode_Check(python_string)) {
auto py_bytes = nb::steal(PyUnicode_AsASCIIString(python_string));
if (py_bytes) {
Expand All @@ -36,16 +34,13 @@ Py2NRNString::Py2NRNString(PyObject* python_string, bool disable_release) {
} else { // Neither Unicode or PyBytes
PyErr_SetString(PyExc_TypeError, "Neither Unicode or PyBytes");
}

return neuron::unique_cstr(str_);
}

void Py2NRNString::set_pyerr(PyObject* type, const char* message) {
nb::object err_type;
nb::object err_value;
nb::object err_traceback;
auto [err_type, err_value, err_traceback] = fetch_pyerr();

if (err()) {
std::tie(err_type, err_value, err_traceback) = fetch_pyerr();
}
if (err_value && err_type) {
auto umes = nb::steal(
PyUnicode_FromFormat("%s (Note: %S: %S)", message, err_type.ptr(), err_value.ptr()));
Expand All @@ -55,48 +50,27 @@ void Py2NRNString::set_pyerr(PyObject* type, const char* message) {
}
}

char* Py2NRNString::get_pyerr() {
if (err()) {
auto [ptype, pvalue, ptraceback] = fetch_pyerr();
if (pvalue) {
auto pstr = nb::steal(PyObject_Str(pvalue.ptr()));
if (pstr) {
const char* err_msg = PyUnicode_AsUTF8(pstr.ptr());
if (err_msg) {
str_ = strdup(err_msg);
} else {
str_ = strdup("get_pyerr failed at PyUnicode_AsUTF8");
}
} else {
str_ = strdup("get_pyerr failed at PyObject_Str");
}
} else {
str_ = strdup("get_pyerr failed at PyErr_Fetch");
}
}
PyErr_Clear(); // in case could not turn pvalue into c_str.
return str_;
}
neuron::unique_cstr Py2NRNString::get_pyerr() {
// Must be called after an error happend.
char* str_ = nullptr;

char* PyErr2NRNString::get_pyerr() {
if (PyErr_Occurred()) {
auto [ptype, pvalue, ptraceback] = fetch_pyerr();
if (pvalue) {
auto pstr = nb::steal(PyObject_Str(pvalue.ptr()));
if (pstr) {
const char* err_msg = PyUnicode_AsUTF8(pstr.ptr());
if (err_msg) {
str_ = strdup(err_msg);
} else {
str_ = strdup("get_pyerr failed at PyUnicode_AsUTF8");
}
auto [ptype, pvalue, ptraceback] = fetch_pyerr();
if (pvalue) {
auto pstr = nb::steal(PyObject_Str(pvalue.ptr()));
if (pstr) {
const char* err_msg = PyUnicode_AsUTF8(pstr.ptr());
if (err_msg) {
str_ = strdup(err_msg);
} else {
str_ = strdup("get_pyerr failed at PyObject_Str");
str_ = strdup("get_pyerr failed at PyUnicode_AsUTF8");
}
} else {
str_ = strdup("get_pyerr failed at PyErr_Fetch");
str_ = strdup("get_pyerr failed at PyObject_Str");
}
} else {
str_ = strdup("get_pyerr failed at PyErr_Fetch");
}

PyErr_Clear(); // in case could not turn pvalue into c_str.
return str_;
return neuron::unique_cstr(str_);
}
Loading

0 comments on commit 9d6872b

Please sign in to comment.