From 7085eeaab415fdeb43a5f77c9383f831a4b85acf Mon Sep 17 00:00:00 2001 From: Philippe Proulx Date: Thu, 27 Jun 2019 12:24:23 -0400 Subject: [PATCH] Add `src/py-common`, containing bt_py_common_format_exception() for now This new convenience library is meant to contain functions used by Python bindings and the Python plugin provider. As of this patch, it only offers bt_py_common_format_exception(), which is similar to what bt2_py_loge_exception() used to do in `native_bt_component_class.i`, except that it returns the formatted exception string instead of logging it directly. Now bt2_py_loge_exception() uses bt_py_common_format_exception(). bt_py_common_format_exception() will be needed in a future patch to set the message of an error cause when appending from the Python plugin provider. No functional change intended. Signed-off-by: Philippe Proulx Change-Id: Iaa9dc081e7d8a3d6c85420fcbbfc045a9d79bf27 Reviewed-on: https://review.lttng.org/c/babeltrace/+/1556 Tested-by: jenkins --- configure.ac | 3 +- src/Makefile.am | 1 + src/bindings/python/bt2/bt2/native_bt.i | 1 + .../bt2/bt2/native_bt_component_class.i | 78 +-------- src/bindings/python/bt2/setup.py.in | 3 +- src/py-common/Makefile.am | 9 ++ src/py-common/py-common.c | 152 ++++++++++++++++++ src/py-common/py-common.h | 46 ++++++ 8 files changed, 220 insertions(+), 73 deletions(-) create mode 100644 src/py-common/Makefile.am create mode 100644 src/py-common/py-common.c create mode 100644 src/py-common/py-common.h diff --git a/configure.ac b/configure.ac index b0434771..74b556f7 100644 --- a/configure.ac +++ b/configure.ac @@ -490,7 +490,7 @@ AM_CONDITIONAL([ENABLE_API_DOC], [test "x$enable_api_doc" = xyes]) AM_CONDITIONAL([ENABLE_BUILT_IN_PLUGINS], [test "x$enable_built_in_plugins" = xyes]) AM_CONDITIONAL([ENABLE_BUILT_IN_PYTHON_PLUGIN_SUPPORT], [test "x$enable_built_in_python_plugin_support" = xyes]) AM_CONDITIONAL([ENABLE_MAN_PAGES], [test "x$enable_man_pages" = xyes]) - +AM_CONDITIONAL([HAVE_PYTHON], [test "x$enable_python_bindings" = xyes || test "x$enable_python_plugins" = xyes]) # Set defines for optionnal features conditionnals in the source code @@ -773,6 +773,7 @@ AC_CONFIG_FILES([ src/plugins/utils/Makefile src/plugins/utils/muxer/Makefile src/plugins/utils/trimmer/Makefile + src/py-common/Makefile src/python-plugin-provider/Makefile tests/ctf-writer/Makefile tests/lib/Makefile diff --git a/src/Makefile.am b/src/Makefile.am index 28b7bd4c..90619a2f 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1,5 +1,6 @@ SUBDIRS = \ common \ + py-common \ ctfser \ fd-cache \ compat \ diff --git a/src/bindings/python/bt2/bt2/native_bt.i b/src/bindings/python/bt2/bt2/native_bt.i index c062f34a..293189d6 100644 --- a/src/bindings/python/bt2/bt2/native_bt.i +++ b/src/bindings/python/bt2/bt2/native_bt.i @@ -35,6 +35,7 @@ #include #include #include "common/assert.h" +#include "py-common/py-common.h" typedef const uint8_t *bt_uuid; %} diff --git a/src/bindings/python/bt2/bt2/native_bt_component_class.i b/src/bindings/python/bt2/bt2/native_bt_component_class.i index bed4602e..18078eea 100644 --- a/src/bindings/python/bt2/bt2/native_bt_component_class.i +++ b/src/bindings/python/bt2/bt2/native_bt_component_class.i @@ -167,88 +167,24 @@ void bt_py3_native_comp_class_dtor(void) { } } - -// TODO: maybe we can wrap code in the Python methods (e.g. _query_from_native) -// in a try catch and print the error there instead, it would be simpler. static void bt2_py_loge_exception(void) { - PyObject *type = NULL; - PyObject *value = NULL; - PyObject *traceback = NULL; - PyObject *traceback_module = NULL; - PyObject *format_exception_func = NULL; - PyObject *exc_str_list = NULL; - GString *msg_buf = NULL; - Py_ssize_t i; + GString *gstr; BT_ASSERT(PyErr_Occurred() != NULL); - - PyErr_Fetch(&type, &value, &traceback); - - BT_ASSERT(type != NULL); - - /* - * traceback can be NULL, when we fail to call a Python function from the - * native code (there is no Python stack at that point). E.g.: - * - * TypeError: _query_from_native() takes 5 positional arguments but 8 were given - */ - - - /* Make sure `value` is what we expected - an instance of `type`. */ - PyErr_NormalizeException(&type, &value, &traceback); - - traceback_module = PyImport_ImportModule("traceback"); - if (!traceback_module) { - BT_LOGE_STR("Failed to log Python exception (could not import traceback module)."); - goto end; - } - - format_exception_func = PyObject_GetAttrString(traceback_module, - traceback ? "format_exception" : "format_exception_only"); - if (!format_exception_func) { - BT_LOGE_STR("Failed to log Python exception (could not find format_exception)."); - goto end; - } - - if (!PyCallable_Check(format_exception_func)) { - BT_LOGE_STR("Failed to log Python exception (format_exception is not callable)."); - goto end; - } - - exc_str_list = PyObject_CallFunctionObjArgs(format_exception_func, type, value, traceback, NULL); - if (!exc_str_list) { - PyErr_Print(); - BT_LOGE_STR("Failed to log Python exception (call to format_exception failed)."); + gstr = bt_py_common_format_exception(BT_LOG_OUTPUT_LEVEL); + if (!gstr) { + /* bt_py_common_format_exception() logs errors */ goto end; } - msg_buf = g_string_new(NULL); - - for (i = 0; i < PyList_Size(exc_str_list); i++) { - PyObject *exc_str = PyList_GetItem(exc_str_list, i); - const char *str = PyUnicode_AsUTF8(exc_str); - if (!str) { - BT_LOGE_STR("Failed to log Python exception (failed to convert exception to string)."); - goto end; - } - - g_string_append(msg_buf, str); - } - - BT_LOGE_STR(msg_buf->str); + BT_LOGE_STR(gstr->str); end: - if (msg_buf) { - g_string_free(msg_buf, TRUE); + if (gstr) { + g_string_free(gstr, TRUE); } - Py_XDECREF(exc_str_list); - Py_XDECREF(format_exception_func); - Py_XDECREF(traceback_module); - - /* PyErr_Restore takes our references. */ - PyErr_Restore(type, value, traceback); } static diff --git a/src/bindings/python/bt2/setup.py.in b/src/bindings/python/bt2/setup.py.in index 78398605..e57ac024 100644 --- a/src/bindings/python/bt2/setup.py.in +++ b/src/bindings/python/bt2/setup.py.in @@ -39,7 +39,8 @@ def main(): sources=['bt2/native_bt.i', 'bt2/logging.c'], libraries=['babeltrace2', 'glib-2.0'], extra_objects=['@top_builddir@/src/logging/.libs/libbabeltrace2-logging.a', - '@top_builddir@/src/common/.libs/libbabeltrace2-common.a'], + '@top_builddir@/src/common/.libs/libbabeltrace2-common.a', + '@top_builddir@/src/py-common/.libs/libbabeltrace2-py-common.a'], swig_opts=['-I@top_srcdir@/include']) dist = setup(name='bt2', diff --git a/src/py-common/Makefile.am b/src/py-common/Makefile.am new file mode 100644 index 00000000..bc52700b --- /dev/null +++ b/src/py-common/Makefile.am @@ -0,0 +1,9 @@ +if HAVE_PYTHON +AM_CPPFLAGS += $(PYTHON_INCLUDE) + +noinst_LTLIBRARIES = libbabeltrace2-py-common.la + +libbabeltrace2_py_common_la_SOURCES = \ + py-common.c \ + py-common.h +endif # HAVE_PYTHON diff --git a/src/py-common/py-common.c b/src/py-common/py-common.c new file mode 100644 index 00000000..ac780034 --- /dev/null +++ b/src/py-common/py-common.c @@ -0,0 +1,152 @@ +/* + * Copyright (c) 2019 EfficiOS Inc. and Linux Foundation + * Copyright (c) 2019 Philippe Proulx + * Copyright (c) 2019 Simon Marchi + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +#define BT_LOG_OUTPUT_LEVEL log_level +#define BT_LOG_TAG "PY-COMMON" +#include "logging/log.h" + +#include + +#include "common/assert.h" +#include "py-common.h" + +BT_HIDDEN +GString *bt_py_common_format_exception(int log_level) +{ + PyObject *type = NULL; + PyObject *value = NULL; + PyObject *traceback = NULL; + PyObject *traceback_module = NULL; + PyObject *format_exception_func = NULL; + PyObject *exc_str_list = NULL; + GString *msg_buf = NULL; + const char *format_exc_func_name; + Py_ssize_t i; + + BT_ASSERT(PyErr_Occurred() != NULL); + PyErr_Fetch(&type, &value, &traceback); + BT_ASSERT(type != NULL); + + /* Make sure `value` is what we expected: an instance of `type` */ + PyErr_NormalizeException(&type, &value, &traceback); + + /* + * Import the standard `traceback` module which contains the + * functions to format an exception. + */ + traceback_module = PyImport_ImportModule("traceback"); + if (!traceback_module) { + BT_LOGE_STR("Failed to import `traceback` module."); + goto error; + } + + /* + * `traceback` can be `NULL`, when we fail to call a Python + * function from native code (there is no Python stack at that + * point). For example, a function which takes 5 positional + * arguments, but 8 were given. + */ + format_exc_func_name = traceback ? "format_exception" : + "format_exception_only"; + format_exception_func = PyObject_GetAttrString(traceback_module, + format_exc_func_name); + if (!format_exception_func) { + BT_LOGE("Cannot find `%s` attribute in `traceback` module.", + format_exc_func_name); + goto error; + } + + if (!PyCallable_Check(format_exception_func)) { + BT_LOGE("`traceback.%s` attribute is not callable.", + format_exc_func_name); + goto error; + } + + exc_str_list = PyObject_CallFunctionObjArgs(format_exception_func, + type, value, traceback, NULL); + if (!exc_str_list) { + if (BT_LOG_ON_ERROR) { + BT_LOGE("Failed to call `traceback.%s` function:", + format_exc_func_name); + PyErr_Print(); + } + + goto error; + } + + msg_buf = g_string_new(NULL); + + for (i = 0; i < PyList_Size(exc_str_list); i++) { + PyObject *exc_str; + const char *str; + + exc_str = PyList_GetItem(exc_str_list, i); + BT_ASSERT(exc_str); + + /* `str` is owned by `exc_str`, not by us */ + str = PyUnicode_AsUTF8(exc_str); + if (!str) { + if (BT_LOG_ON_ERROR) { + BT_LOGE_STR("PyUnicode_AsUTF8() failed:"); + PyErr_Print(); + } + + goto error; + } + + /* `str` has a trailing newline */ + g_string_append(msg_buf, str); + } + + if (msg_buf->len > 0) { + /* Remove trailing newline if any */ + if (msg_buf->str[msg_buf->len - 1] == '\n') { + g_string_truncate(msg_buf, msg_buf->len - 1); + } + } + + goto end; + +error: + if (msg_buf) { + g_string_free(msg_buf, TRUE); + msg_buf = NULL; + } + +end: + Py_XDECREF(exc_str_list); + Py_XDECREF(format_exception_func); + Py_XDECREF(traceback_module); + + /* + * We can safely call PyErr_Restore() because we always call + * PyErr_Fetch(), and having an error indicator is a function's + * precondition. + * + * PyErr_Restore() steals our references. + */ + PyErr_Restore(type, value, traceback); + + return msg_buf; +} diff --git a/src/py-common/py-common.h b/src/py-common/py-common.h new file mode 100644 index 00000000..dbfd8825 --- /dev/null +++ b/src/py-common/py-common.h @@ -0,0 +1,46 @@ +#ifndef BABELTRACE_PY_COMMON_INTERNAL_H +#define BABELTRACE_PY_COMMON_INTERNAL_H + +/* + * Copyright (c) 2019 EfficiOS Inc. and Linux Foundation + * Copyright (c) 2019 Philippe Proulx + * Copyright (c) 2019 Simon Marchi + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +#include + +#include "common/macros.h" + +/* + * Formats the Python error indicator (exception) and returns the + * formatted string, or `NULL` on error. The returned string does NOT + * end with a newline. + * + * You must ensure that the error indicator is set with PyErr_Occurred() + * before you call this function. + * + * This function does not modify the error indicator, that is, anything + * that is fetched is always restored. + */ +BT_HIDDEN +GString *bt_py_common_format_exception(int log_level); + +#endif /* BABELTRACE_PY_COMMON_INTERNAL_H */ -- 2.34.1