Re: [Libguestfs] [libnbd PATCH v3 1/5] python: Simplify passing of mutable *error to callbacks

Instead of open-coding our code to create a PyObject wrapper of the mutable *error to be passed to each callback, extract this into a helper function. We can then slightly optimize things to hang on to a single pointer to the ctypes module, rather than looking it up on every callback. The generated code shrinks by more than the added code in utils.c: | --- python/methods.h.bak 2022-06-08 14:35:00.454786844 -0500 | +++ python/methods.h 2022-06-08 14:43:24.681596853 -0500 | @@ -35,6 +35,7 @@ | extern PyObject *nbd_internal_py_get_aio_view (PyObject *, bool); | extern int nbd_internal_py_init_aio_buffer (PyObject *); | extern PyObject *nbd_internal_py_get_nbd_buffer_type (void); | +extern PyObject *nbd_internal_py_wrap_errptr (int); | | static inline struct nbd_handle * | get_handle (PyObject *obj) | --- python/methods.c.bak 2022-06-08 14:35:00.450786838 -0500 | +++ python/methods.c 2022-06-08 14:43:24.696596877 -0500 | @@ -75,13 +75,7 @@ chunk_wrapper (void *user_data, const vo | PyObject *py_args, *py_ret; | PyObject *py_error = NULL; | | - PyObject *py_error_modname = PyUnicode_FromString ("ctypes"); | - if (!py_error_modname) { PyErr_PrintEx (0); goto out; } | - PyObject *py_error_mod = PyImport_Import (py_error_modname); | - Py_DECREF (py_error_modname); | - if (!py_error_mod) { PyErr_PrintEx (0); goto out; } | - py_error = PyObject_CallMethod (py_error_mod, "c_int", "i", *error); | - Py_DECREF (py_error_mod); | + py_error = nbd_internal_py_wrap_errptr (*error); | if (!py_error) { PyErr_PrintEx (0); goto out; } | | py_args = Py_BuildValue ("(y#KIO)", subbuf, (int) count, offset, status, | @@ -132,13 +126,7 @@ completion_wrapper (void *user_data, int | PyObject *py_args, *py_ret; | PyObject *py_error = NULL; | | - PyObject *py_error_modname = PyUnicode_FromString ("ctypes"); | - if (!py_error_modname) { PyErr_PrintEx (0); goto out; } | - PyObject *py_error_mod = PyImport_Import (py_error_modname); | - Py_DECREF (py_error_modname); | - if (!py_error_mod) { PyErr_PrintEx (0); goto out; } | - py_error = PyObject_CallMethod (py_error_mod, "c_int", "i", *error); | - Py_DECREF (py_error_mod); | + py_error = nbd_internal_py_wrap_errptr (*error); | if (!py_error) { PyErr_PrintEx (0); goto out; } | | py_args = Py_BuildValue ("(O)", py_error); | @@ -239,13 +227,7 @@ extent_wrapper (void *user_data, const c | if (!py_e_entries) { PyErr_PrintEx (0); goto out; } | PyList_SET_ITEM (py_entries, i_entries, py_e_entries); | } | - PyObject *py_error_modname = PyUnicode_FromString ("ctypes"); | - if (!py_error_modname) { PyErr_PrintEx (0); goto out; } | - PyObject *py_error_mod = PyImport_Import (py_error_modname); | - Py_DECREF (py_error_modname); | - if (!py_error_mod) { PyErr_PrintEx (0); goto out; } | - py_error = PyObject_CallMethod (py_error_mod, "c_int", "i", *error); | - Py_DECREF (py_error_mod); | + py_error = nbd_internal_py_wrap_errptr (*error); | if (!py_error) { PyErr_PrintEx (0); goto out; } | | py_args = Py_BuildValue ("(sKOO)", metacontext, offset, py_entries, --- generator/Python.ml | 9 ++------- python/utils.c | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/generator/Python.ml b/generator/Python.ml index e94f37b..6980034 100644 --- a/generator/Python.ml +++ b/generator/Python.ml @@ -41,6 +41,7 @@ let extern PyObject *nbd_internal_py_get_aio_view (PyObject *, bool); extern int nbd_internal_py_init_aio_buffer (PyObject *); extern PyObject *nbd_internal_py_get_nbd_buffer_type (void); +extern PyObject *nbd_internal_py_wrap_errptr (int); static inline struct nbd_handle * get_handle (PyObject *obj) @@ -184,13 +185,7 @@ let | CBInt _ | CBInt64 _ -> () | CBMutable (Int n) -> - pr " PyObject *py_%s_modname = PyUnicode_FromString (\"ctypes\");\n" n; - pr " if (!py_%s_modname) { PyErr_PrintEx (0); goto out; }\n" n; - pr " PyObject *py_%s_mod = PyImport_Import (py_%s_modname);\n" n n; - pr " Py_DECREF (py_%s_modname);\n" n; - pr " if (!py_%s_mod) { PyErr_PrintEx (0); goto out; }\n" n; - pr " py_%s = PyObject_CallMethod (py_%s_mod, \"c_int\", \"i\", *%s);\n" n n n; - pr " Py_DECREF (py_%s_mod);\n" n; + pr " py_%s = nbd_internal_py_wrap_errptr (*%s);\n" n n; pr " if (!py_%s) { PyErr_PrintEx (0); goto out; }\n" n; | CBString _ | CBUInt _ diff --git a/python/utils.c b/python/utils.c index e0df181..cd44b90 100644 --- a/python/utils.c +++ b/python/utils.c @@ -173,3 +173,22 @@ nbd_internal_py_get_nbd_buffer_type (void) } return type; } + +/* Helper to package callback *error into modifiable PyObject */ +PyObject * +nbd_internal_py_wrap_errptr (int err) +{ + static PyObject *py_ctypes_mod; + + if (!py_ctypes_mod) { + PyObject *py_modname = PyUnicode_FromString ("ctypes"); + if (!py_modname) + return NULL; + py_ctypes_mod = PyImport_Import (py_modname); + Py_DECREF (py_modname); + if (!py_ctypes_mod) + return NULL; + } + + return PyObject_CallMethod (py_ctypes_mod, "c_int", "i", err); +}