This patch changes some functions in the Python bindings native code to
use some error handling patterns that are used elsewhere in the project.
I think they help avoid some mistakes, and always using the same
patterns helps to spot mistakes more easily.
- Don't initialize status/ret variables to OK when declaring them. If
we do this and forget to assign them in some error path, we'll
wrongfully return OK. By not initializing them, if we forget to set
them on some error path, the compilers or static analyzers will be able
to identify it as a problem because the variable will be used without
being initialized on that path.
- When checking for Python API calls failure, check if the returned
pointer is NULL instead of checking PyErr_Occurred(). It is equivalent,
but it's preferable to do it the same way everywhere.
- After each API call, have an error check with a "goto end;".
Change-Id: I54e62a77a0a9ab3d778edceb85cbf280b134db88
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2223
Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com>
bt_self_message_iterator *self_message_iterator,
const char *module_name, int active_log_level)
{
bt_self_message_iterator *self_message_iterator,
const char *module_name, int active_log_level)
{
- int status = __BT_FUNC_STATUS_OK;
PyObject *exc = PyErr_Occurred();
if (!exc) {
PyObject *exc = PyErr_Occurred();
if (!exc) {
+ status = __BT_FUNC_STATUS_OK;
{
const bt_component *component = bt_self_component_as_component(self_component);
const bt_component_class *component_class = bt_component_borrow_class_const(component);
{
const bt_component *component = bt_self_component_as_component(self_component);
const bt_component_class *component_class = bt_component_borrow_class_const(component);
- bt_component_class_initialize_method_status status = __BT_FUNC_STATUS_OK;
+ bt_component_class_initialize_method_status status;
PyObject *py_cls = NULL;
PyObject *py_comp = NULL;
PyObject *py_params_ptr = NULL;
PyObject *py_cls = NULL;
PyObject *py_comp = NULL;
PyObject *py_params_ptr = NULL;
*/
bt_self_component_set_data(self_component, py_comp);
py_comp = NULL;
*/
bt_self_component_set_data(self_component, py_comp);
py_comp = NULL;
+
+ status = BT_COMPONENT_CLASS_INITIALIZE_METHOD_STATUS_OK;
+
- status = __BT_FUNC_STATUS_ERROR;
-
- /*
- * Clear any exception: we're returning a bad status anyway. If
- * this call originated from Python (creation from a plugin's
- * component class, for example), then the user gets an
- * appropriate creation error.
- */
- PyErr_Clear();
+ /* This error path is for non-Python errors only. */
+ status = BT_COMPONENT_CLASS_INITIALIZE_METHOD_STATUS_ERROR;
+ BT_ASSERT(!PyErr_Occurred());
Py_XDECREF(py_comp);
Py_XDECREF(py_params_ptr);
Py_XDECREF(py_comp_ptr);
Py_XDECREF(py_comp);
Py_XDECREF(py_params_ptr);
Py_XDECREF(py_comp_ptr);
PyObject *py_params_ptr = NULL;
PyObject *py_range_set_addr = NULL;
bt_integer_range_set_unsigned *ret_range_set = NULL;
PyObject *py_params_ptr = NULL;
PyObject *py_range_set_addr = NULL;
bt_integer_range_set_unsigned *ret_range_set = NULL;
- bt_component_class_get_supported_mip_versions_method_status status =
- __BT_FUNC_STATUS_OK;
+ bt_component_class_get_supported_mip_versions_method_status status;
py_cls = lookup_cc_ptr_to_py_cls(component_class);
if (!py_cls) {
py_cls = lookup_cc_ptr_to_py_cls(component_class);
if (!py_cls) {
+ status = BT_COMPONENT_CLASS_GET_SUPPORTED_MIP_VERSIONS_METHOD_STATUS_OK;
+
- PyErr_Clear();
- status = __BT_FUNC_STATUS_ERROR;
+ /* This error path is for non-Python errors only. */
+ status = BT_COMPONENT_CLASS_GET_SUPPORTED_MIP_VERSIONS_METHOD_STATUS_ERROR;
+ BT_ASSERT(!PyErr_Occurred());
Py_XDECREF(py_params_ptr);
Py_XDECREF(py_range_set_addr);
bt_integer_range_set_unsigned_put_ref(ret_range_set);
Py_XDECREF(py_params_ptr);
Py_XDECREF(py_range_set_addr);
bt_integer_range_set_unsigned_put_ref(ret_range_set);
void component_class_finalize(bt_self_component *self_component)
{
PyObject *py_comp = bt_self_component_get_data(self_component);
void component_class_finalize(bt_self_component *self_component)
{
PyObject *py_comp = bt_self_component_get_data(self_component);
+ PyObject *py_method_result;
+
BT_ASSERT(py_comp);
/* Call user's _user_finalize() method */
BT_ASSERT(py_comp);
/* Call user's _user_finalize() method */
- PyObject *py_method_result = PyObject_CallMethod(py_comp,
- "_user_finalize", NULL);
-
- if (PyErr_Occurred()) {
+ py_method_result = PyObject_CallMethod(py_comp, "_user_finalize", NULL);
+ if (!py_method_result) {
bt_logging_level log_level = get_self_component_log_level(
self_component);
BT_COMP_LOG_CUR_LVL(BT_LOG_WARNING, log_level, self_component,
"User component's _user_finalize() method raised an exception: ignoring:");
logw_exception(log_level);
bt_logging_level log_level = get_self_component_log_level(
self_component);
BT_COMP_LOG_CUR_LVL(BT_LOG_WARNING, log_level, self_component,
"User component's _user_finalize() method raised an exception: ignoring:");
logw_exception(log_level);
+
+ /*
+ * Ignore any exception raised by the _user_finalize() method
+ * because it won't change anything at this point: the component
+ * is being destroyed anyway.
+ */
+ PyErr_Clear();
+ goto end;
- /*
- * Ignore any exception raised by the _user_finalize() method
- * because it won't change anything at this point: the component
- * is being destroyed anyway.
- */
- PyErr_Clear();
+ BT_ASSERT(py_method_result == Py_None);
+
+end:
Py_XDECREF(py_method_result);
Py_DECREF(py_comp);
}
Py_XDECREF(py_method_result);
Py_DECREF(py_comp);
}
PyObject *py_result = NULL;
bt_component_class_message_iterator_can_seek_beginning_method_status status;
py_iter = bt_self_message_iterator_get_data(self_message_iterator);
PyObject *py_result = NULL;
bt_component_class_message_iterator_can_seek_beginning_method_status status;
py_iter = bt_self_message_iterator_get_data(self_message_iterator);
BT_ASSERT(py_iter);
py_result = PyObject_CallMethod(py_iter,
"_bt_can_seek_beginning_from_native", NULL);
BT_ASSERT(py_iter);
py_result = PyObject_CallMethod(py_iter,
"_bt_can_seek_beginning_from_native", NULL);
-
- BT_ASSERT(!py_result || PyBool_Check(py_result));
-
- if (py_result) {
- *can_seek = PyObject_IsTrue(py_result);
- status = BT_COMPONENT_CLASS_MESSAGE_ITERATOR_CAN_SEEK_BEGINNING_METHOD_STATUS_OK;
- } else {
status = py_exc_to_status_message_iterator_clear(self_message_iterator);
status = py_exc_to_status_message_iterator_clear(self_message_iterator);
- BT_ASSERT(status != __BT_FUNC_STATUS_OK);
+ BT_ASSERT(PyBool_Check(py_result));
+ *can_seek = PyObject_IsTrue(py_result);
+
+ status = BT_COMPONENT_CLASS_MESSAGE_ITERATOR_CAN_SEEK_BEGINNING_METHOD_STATUS_OK;
+
+end:
Py_XDECREF(py_result);
return status;
Py_XDECREF(py_result);
return status;
py_iter = bt_self_message_iterator_get_data(self_message_iterator);
BT_ASSERT(py_iter);
py_iter = bt_self_message_iterator_get_data(self_message_iterator);
BT_ASSERT(py_iter);
- py_result = PyObject_CallMethod(py_iter, "_bt_seek_beginning_from_native",
+
+ py_result = PyObject_CallMethod(py_iter,
+ "_bt_seek_beginning_from_native",
- BT_ASSERT(!py_result || py_result == Py_None);
- status = py_exc_to_status_message_iterator_clear(self_message_iterator);
+ if (!py_result) {
+ status = py_exc_to_status_message_iterator_clear(self_message_iterator);
+ goto end;
+ }
+
+ BT_ASSERT(py_result == Py_None);
+
+ status = BT_COMPONENT_CLASS_MESSAGE_ITERATOR_SEEK_BEGINNING_METHOD_STATUS_OK;
+
+end:
PyObject *py_iter;
PyObject *py_result = NULL;
bt_component_class_message_iterator_can_seek_ns_from_origin_method_status status;
PyObject *py_iter;
PyObject *py_result = NULL;
bt_component_class_message_iterator_can_seek_ns_from_origin_method_status status;
py_iter = bt_self_message_iterator_get_data(self_message_iterator);
BT_ASSERT(py_iter);
py_result = PyObject_CallMethod(py_iter,
"_bt_can_seek_ns_from_origin_from_native", "L", ns_from_origin);
py_iter = bt_self_message_iterator_get_data(self_message_iterator);
BT_ASSERT(py_iter);
py_result = PyObject_CallMethod(py_iter,
"_bt_can_seek_ns_from_origin_from_native", "L", ns_from_origin);
-
- BT_ASSERT(!py_result || PyBool_Check(py_result));
-
- if (py_result) {
- *can_seek = PyObject_IsTrue(py_result);
- status = BT_COMPONENT_CLASS_MESSAGE_ITERATOR_CAN_SEEK_NS_FROM_ORIGIN_METHOD_STATUS_OK;
- } else {
status = py_exc_to_status_message_iterator_clear(self_message_iterator);
status = py_exc_to_status_message_iterator_clear(self_message_iterator);
- BT_ASSERT(status != BT_COMPONENT_CLASS_MESSAGE_ITERATOR_CAN_SEEK_NS_FROM_ORIGIN_METHOD_STATUS_OK);
+ BT_ASSERT(PyBool_Check(py_result));
+ *can_seek = PyObject_IsTrue(py_result);
+
+ status = BT_COMPONENT_CLASS_MESSAGE_ITERATOR_CAN_SEEK_NS_FROM_ORIGIN_METHOD_STATUS_OK;
+
+end:
Py_XDECREF(py_result);
return status;
Py_XDECREF(py_result);
return status;
py_iter = bt_self_message_iterator_get_data(self_message_iterator);
BT_ASSERT(py_iter);
py_iter = bt_self_message_iterator_get_data(self_message_iterator);
BT_ASSERT(py_iter);
py_result = PyObject_CallMethod(py_iter,
"_bt_seek_ns_from_origin_from_native", "L", ns_from_origin);
py_result = PyObject_CallMethod(py_iter,
"_bt_seek_ns_from_origin_from_native", "L", ns_from_origin);
- BT_ASSERT(!py_result || py_result == Py_None);
- status = py_exc_to_status_message_iterator_clear(self_message_iterator);
+ if (!py_result) {
+ status = py_exc_to_status_message_iterator_clear(self_message_iterator);
+ goto end;
+ }
+
+
+ BT_ASSERT(py_result == Py_None);
+
+ status = BT_COMPONENT_CLASS_MESSAGE_ITERATOR_SEEK_NS_FROM_ORIGIN_METHOD_STATUS_OK;
+
+end:
BT_COMP_LOG_CUR_LVL(BT_LOG_ERROR, log_level, self_component,
"Failed to create a SWIG pointer object.");
status = __BT_FUNC_STATUS_MEMORY_ERROR;
BT_COMP_LOG_CUR_LVL(BT_LOG_ERROR, log_level, self_component,
"Failed to create a SWIG pointer object.");
status = __BT_FUNC_STATUS_MEMORY_ERROR;
py_method_result = PyObject_CallMethod(py_comp,
"_bt_port_connected_from_native", "(OiO)", py_self_port_ptr,
self_component_port_type, py_other_port_ptr);
py_method_result = PyObject_CallMethod(py_comp,
"_bt_port_connected_from_native", "(OiO)", py_self_port_ptr,
self_component_port_type, py_other_port_ptr);
- BT_ASSERT(!py_method_result || py_method_result == Py_None);
- status = py_exc_to_status_component_clear(self_component);
+ if (!py_method_result) {
+ status = py_exc_to_status_component_clear(self_component);
+ goto end;
+ }
+
+ BT_ASSERT(py_method_result == Py_None);
+
+ status = BT_COMPONENT_CLASS_PORT_CONNECTED_METHOD_STATUS_OK;
end:
Py_XDECREF(py_self_port_ptr);
Py_XDECREF(py_other_port_ptr);
Py_XDECREF(py_method_result);
end:
Py_XDECREF(py_self_port_ptr);
Py_XDECREF(py_other_port_ptr);
Py_XDECREF(py_method_result);
{
PyObject *py_comp = NULL;
PyObject *py_method_result = NULL;
{
PyObject *py_comp = NULL;
PyObject *py_method_result = NULL;
- bt_component_class_sink_graph_is_configured_method_status status = __BT_FUNC_STATUS_OK;
+ bt_component_class_sink_graph_is_configured_method_status status;
bt_self_component *self_component = bt_self_component_sink_as_self_component(self_component_sink);
py_comp = bt_self_component_get_data(self_component);
bt_self_component *self_component = bt_self_component_sink_as_self_component(self_component_sink);
py_comp = bt_self_component_get_data(self_component);
py_method_result = PyObject_CallMethod(py_comp,
"_bt_graph_is_configured_from_native", NULL);
py_method_result = PyObject_CallMethod(py_comp,
"_bt_graph_is_configured_from_native", NULL);
- BT_ASSERT(!py_method_result || py_method_result == Py_None);
- status = py_exc_to_status_component_clear(self_component);
+ if (!py_method_result) {
+ status = py_exc_to_status_component_clear(self_component);
+ goto end;
+ }
+
+ BT_ASSERT(py_method_result == Py_None);
+
+ status = BT_COMPONENT_CLASS_SINK_GRAPH_IS_CONFIGURED_METHOD_STATUS_OK;;
+
+end:
Py_XDECREF(py_method_result);
return status;
}
Py_XDECREF(py_method_result);
return status;
}
python_error:
/* Handling of errors that cause a Python exception to be set. */
status = py_exc_to_status_message_iterator_clear(self_message_iterator);
python_error:
/* Handling of errors that cause a Python exception to be set. */
status = py_exc_to_status_message_iterator_clear(self_message_iterator);
- BT_ASSERT(status != __BT_FUNC_STATUS_OK);
/* Call user's _user_finalize() method */
py_method_result = PyObject_CallMethod(py_message_iter,
"_user_finalize", NULL);
/* Call user's _user_finalize() method */
py_method_result = PyObject_CallMethod(py_message_iter,
"_user_finalize", NULL);
-
- if (PyErr_Occurred()) {
+ if (!py_method_result) {
bt_self_component *self_comp =
bt_self_message_iterator_borrow_component(
message_iterator);
bt_self_component *self_comp =
bt_self_message_iterator_borrow_component(
message_iterator);
"User's _user_finalize() method raised an exception: ignoring:");
logw_exception(get_self_message_iterator_log_level(
message_iterator));
"User's _user_finalize() method raised an exception: ignoring:");
logw_exception(get_self_message_iterator_log_level(
message_iterator));
+
+ /*
+ * Ignore any exception raised by the _user_finalize() method
+ * because it won't change anything at this point: the component
+ * is being destroyed anyway.
+ */
+ PyErr_Clear();
- /*
- * Ignore any exception raised by the _user_finalize() method
- * because it won't change anything at this point: the component
- * is being destroyed anyway.
- */
- PyErr_Clear();
Py_XDECREF(py_method_result);
Py_DECREF(py_message_iter);
}
Py_XDECREF(py_method_result);
Py_DECREF(py_message_iter);
}
bt_message_array_const msgs, uint64_t capacity,
uint64_t *count)
{
bt_message_array_const msgs, uint64_t capacity,
uint64_t *count)
{
- bt_component_class_message_iterator_next_method_status status = __BT_FUNC_STATUS_OK;
+ bt_component_class_message_iterator_next_method_status status;
PyObject *py_message_iter = bt_self_message_iterator_get_data(message_iterator);
PyObject *py_method_result = NULL;
BT_ASSERT(py_message_iter);
PyObject *py_message_iter = bt_self_message_iterator_get_data(message_iterator);
PyObject *py_method_result = NULL;
BT_ASSERT(py_message_iter);
py_method_result = PyObject_CallMethod(py_message_iter,
"_bt_next_from_native", NULL);
if (!py_method_result) {
status = py_exc_to_status_message_iterator_clear(message_iterator);
py_method_result = PyObject_CallMethod(py_message_iter,
"_bt_next_from_native", NULL);
if (!py_method_result) {
status = py_exc_to_status_message_iterator_clear(message_iterator);
- BT_ASSERT(status != __BT_FUNC_STATUS_OK);
msgs[0] = PyLong_AsVoidPtr(py_method_result);
*count = 1;
msgs[0] = PyLong_AsVoidPtr(py_method_result);
*count = 1;
- /* Clear potential overflow error; should never happen */
+ /* Overflow errors should never happen. */
BT_ASSERT(!PyErr_Occurred());
BT_ASSERT(!PyErr_Occurred());
+
+ status = BT_COMPONENT_CLASS_MESSAGE_ITERATOR_NEXT_METHOD_STATUS_OK;
end:
Py_XDECREF(py_method_result);
end:
Py_XDECREF(py_method_result);
bt_component_class_sink_consume_method_status status;
BT_ASSERT(py_comp);
bt_component_class_sink_consume_method_status status;
BT_ASSERT(py_comp);
py_method_result = PyObject_CallMethod(py_comp,
"_user_consume", NULL);
py_method_result = PyObject_CallMethod(py_comp,
"_user_consume", NULL);
- status = py_exc_to_status_component_clear(self_component);
- BT_ASSERT(py_method_result || status != __BT_FUNC_STATUS_OK);
+ if (!py_method_result) {
+ status = py_exc_to_status_component_clear(self_component);
+ goto end;
+ }
+
+ status = BT_COMPONENT_CLASS_SINK_CONSUME_METHOD_STATUS_OK;
+
+end:
Py_XDECREF(py_method_result);
return status;
}
Py_XDECREF(py_method_result);
return status;
}
PyTuple_SET_ITEM(py_listener_ids, 3, py_listener_id);
py_listener_id = NULL;
PyTuple_SET_ITEM(py_listener_ids, 3, py_listener_id);
py_listener_id = NULL;
Py_INCREF(py_listener_ids);
end:
Py_INCREF(py_listener_ids);
end:
Py_XDECREF(py_listener_id);
return py_listener_ids;
}
Py_XDECREF(py_listener_id);
return py_listener_ids;
}
}
py_res = PyObject_CallFunction(py_callable, "(O)", py_trace_ptr);
}
py_res = PyObject_CallFunction(py_callable, "(O)", py_trace_ptr);
- if (py_res) {
- BT_ASSERT(py_res == Py_None);
- } else {
loge_exception_append_cause(
"Trace's destruction listener (Python)",
BT_LOG_OUTPUT_LEVEL);
loge_exception_append_cause(
"Trace's destruction listener (Python)",
BT_LOG_OUTPUT_LEVEL);
+ BT_ASSERT(py_res == Py_None);
+
+end:
Py_DECREF(py_trace_ptr);
Py_XDECREF(py_res);
}
Py_DECREF(py_trace_ptr);
Py_XDECREF(py_res);
}
}
py_res = PyObject_CallFunction(py_callable, "(O)", py_trace_class_ptr);
}
py_res = PyObject_CallFunction(py_callable, "(O)", py_trace_class_ptr);
- if (py_res) {
- BT_ASSERT(py_res == Py_None);
- } else {
loge_exception_append_cause(
"Trace class's destruction listener (Python)",
BT_LOG_OUTPUT_LEVEL);
loge_exception_append_cause(
"Trace class's destruction listener (Python)",
BT_LOG_OUTPUT_LEVEL);
+ BT_ASSERT(py_res == Py_None);
+
+end:
Py_DECREF(py_trace_class_ptr);
Py_XDECREF(py_res);
}
Py_DECREF(py_trace_class_ptr);
Py_XDECREF(py_res);
}
BT_ASSERT(trace_class);
BT_ASSERT(py_callable);
BT_ASSERT(trace_class);
BT_ASSERT(py_callable);
status = bt_trace_class_add_destruction_listener(
trace_class, trace_class_destroyed_listener, py_callable, id);
if (status == __BT_FUNC_STATUS_OK) {
status = bt_trace_class_add_destruction_listener(
trace_class, trace_class_destroyed_listener, py_callable, id);
if (status == __BT_FUNC_STATUS_OK) {