From a91462c9af36b77653794c73ce6fbed0c8ba0b2d Mon Sep 17 00:00:00 2001 From: Philippe Proulx Date: Sun, 12 Jan 2020 10:45:31 -0500 Subject: [PATCH] lib: graph API: return borrowed references when adding to an object Before this patch, the following functions return a new reference when their last parameter is not `NULL`: * bt_graph_add_filter_component() * bt_graph_add_filter_component_with_initialize_method_data() * bt_graph_add_simple_sink_component() * bt_graph_add_sink_component() * bt_graph_add_sink_component_with_initialize_method_data() * bt_graph_add_source_component() * bt_graph_add_source_component_with_initialize_method_data() * bt_graph_connect_ports() * bt_self_component_filter_add_input_port() * bt_self_component_filter_add_output_port() * bt_self_component_sink_add_input_port() * bt_self_component_source_add_output_port() I'm changing this so that they return a borrowed reference instead. This is more in line with other non-creating functions which always return borrowed references. It's okay to borrow here because the object to which you add an object becomes its owner anyway. Most sites are updated by removing the *_put_ref() call as the reference is now borrowed. Signed-off-by: Philippe Proulx Change-Id: I71a5e18760504d8f8610162e3f6d7bd8d87474f9 Reviewed-on: https://review.lttng.org/c/babeltrace/+/2762 Tested-by: jenkins Reviewed-by: Simon Marchi --- src/bindings/python/bt2/bt2/component.py | 16 ++++++++++++---- src/bindings/python/bt2/bt2/graph.py | 6 ++++-- src/cli/babeltrace2.c | 4 ++++ src/lib/graph/component-filter.c | 2 -- src/lib/graph/component-sink.c | 1 - src/lib/graph/component-source.c | 1 - src/lib/graph/graph.c | 2 -- tests/lib/plugin.c | 1 - tests/lib/test_graph_topo.c | 10 ---------- ...estruction_listener_in_destruction_listener.c | 1 - tests/lib/test_simple_sink.c | 2 -- 11 files changed, 20 insertions(+), 26 deletions(-) diff --git a/src/bindings/python/bt2/bt2/component.py b/src/bindings/python/bt2/bt2/component.py index afc1047d..f4aa8e80 100644 --- a/src/bindings/python/bt2/bt2/component.py +++ b/src/bindings/python/bt2/bt2/component.py @@ -845,7 +845,9 @@ class _UserSourceComponent(_UserComponent, _SourceComponentConst): comp_status, 'cannot add output port to source component object' ) assert self_port_ptr is not None - return bt2_port._UserComponentOutputPort._create_from_ptr(self_port_ptr) + return bt2_port._UserComponentOutputPort._create_from_ptr_and_get_ref( + self_port_ptr + ) class _UserFilterComponent(_UserComponent, _FilterComponentConst): @@ -893,7 +895,9 @@ class _UserFilterComponent(_UserComponent, _FilterComponentConst): comp_status, 'cannot add output port to filter component object' ) assert self_port_ptr - return bt2_port._UserComponentOutputPort._create_from_ptr(self_port_ptr) + return bt2_port._UserComponentOutputPort._create_from_ptr_and_get_ref( + self_port_ptr + ) def _add_input_port(self, name, user_data=None): utils._check_str(name) @@ -903,7 +907,9 @@ class _UserFilterComponent(_UserComponent, _FilterComponentConst): comp_status, 'cannot add input port to filter component object' ) assert self_port_ptr - return bt2_port._UserComponentInputPort._create_from_ptr(self_port_ptr) + return bt2_port._UserComponentInputPort._create_from_ptr_and_get_ref( + self_port_ptr + ) class _UserSinkComponent(_UserComponent, _SinkComponentConst): @@ -943,7 +949,9 @@ class _UserSinkComponent(_UserComponent, _SinkComponentConst): comp_status, 'cannot add input port to sink component object' ) assert self_port_ptr - return bt2_port._UserComponentInputPort._create_from_ptr(self_port_ptr) + return bt2_port._UserComponentInputPort._create_from_ptr_and_get_ref( + self_port_ptr + ) def _create_message_iterator(self, input_port): utils._check_type(input_port, bt2_port._UserComponentInputPort) diff --git a/src/bindings/python/bt2/bt2/graph.py b/src/bindings/python/bt2/bt2/graph.py index 9d5ba3aa..9611c6e2 100644 --- a/src/bindings/python/bt2/bt2/graph.py +++ b/src/bindings/python/bt2/bt2/graph.py @@ -119,7 +119,9 @@ class Graph(object._SharedObject): ) utils._handle_func_status(status, 'cannot add component to graph') assert comp_ptr - return bt2_component._create_component_from_const_ptr(comp_ptr, cc_type) + return bt2_component._create_component_from_const_ptr_and_get_ref( + comp_ptr, cc_type + ) def connect_ports(self, upstream_port, downstream_port): utils._check_type(upstream_port, bt2_port._OutputPortConst) @@ -129,7 +131,7 @@ class Graph(object._SharedObject): ) utils._handle_func_status(status, 'cannot connect component ports within graph') assert conn_ptr - return bt2_connection._ConnectionConst._create_from_ptr(conn_ptr) + return bt2_connection._ConnectionConst._create_from_ptr_and_get_ref(conn_ptr) def add_port_added_listener(self, listener): if not callable(listener): diff --git a/src/cli/babeltrace2.c b/src/cli/babeltrace2.c index 0dfac7a8..d065075e 100644 --- a/src/cli/babeltrace2.c +++ b/src/cli/babeltrace2.c @@ -1442,6 +1442,7 @@ int cmd_run_ctx_connect_upstream_port_to_downstream_component( ctx->graph, trimmer_class, trimmer_name, trimmer_params, ctx->cfg->log_level, &trimmer); + bt_component_filter_get_ref(trimmer); free(trimmer_name); if (add_comp_status != BT_GRAPH_ADD_COMPONENT_STATUS_OK) { @@ -2270,18 +2271,21 @@ int cmd_run_ctx_create_components_from_config_components( comp_cls, cfg_comp->instance_name->str, cfg_comp->params, cfg_comp->log_level, (void *) &comp); + bt_component_source_get_ref(comp); break; case BT_COMPONENT_CLASS_TYPE_FILTER: ret = bt_graph_add_filter_component(ctx->graph, comp_cls, cfg_comp->instance_name->str, cfg_comp->params, cfg_comp->log_level, (void *) &comp); + bt_component_filter_get_ref(comp); break; case BT_COMPONENT_CLASS_TYPE_SINK: ret = bt_graph_add_sink_component(ctx->graph, comp_cls, cfg_comp->instance_name->str, cfg_comp->params, cfg_comp->log_level, (void *) &comp); + bt_component_sink_get_ref(comp); break; default: bt_common_abort(); diff --git a/src/lib/graph/component-filter.c b/src/lib/graph/component-filter.c index 9950a502..e805f8da 100644 --- a/src/lib/graph/component-filter.c +++ b/src/lib/graph/component-filter.c @@ -135,7 +135,6 @@ enum bt_self_component_add_port_status bt_self_component_filter_add_output_port( if (self_port) { /* Move reference to user */ *self_port = (void *) port; - port = NULL; } end: @@ -205,7 +204,6 @@ enum bt_self_component_add_port_status bt_self_component_filter_add_input_port( if (self_port) { /* Move reference to user */ *self_port = (void *) port; - port = NULL; } end: diff --git a/src/lib/graph/component-sink.c b/src/lib/graph/component-sink.c index 577239ac..12ea8c27 100644 --- a/src/lib/graph/component-sink.c +++ b/src/lib/graph/component-sink.c @@ -139,7 +139,6 @@ enum bt_self_component_add_port_status bt_self_component_sink_add_input_port( if (self_port) { /* Move reference to user */ *self_port = (void *) port; - port = NULL; } end: diff --git a/src/lib/graph/component-source.c b/src/lib/graph/component-source.c index d793c192..0e3eedc2 100644 --- a/src/lib/graph/component-source.c +++ b/src/lib/graph/component-source.c @@ -133,7 +133,6 @@ enum bt_self_component_add_port_status bt_self_component_source_add_output_port( if (self_port) { /* Move reference to user */ *self_port = (void *) port; - port = NULL; } end: diff --git a/src/lib/graph/graph.c b/src/lib/graph/graph.c index 10080b95..9e7e8ffc 100644 --- a/src/lib/graph/graph.c +++ b/src/lib/graph/graph.c @@ -433,7 +433,6 @@ enum bt_graph_connect_ports_status bt_graph_connect_ports( if (user_connection) { /* Move reference to user */ *user_connection = connection; - connection = NULL; } end: @@ -1020,7 +1019,6 @@ int add_component_with_init_method_data( if (user_component) { /* Move reference to user */ *user_component = component; - component = NULL; } end: diff --git a/tests/lib/plugin.c b/tests/lib/plugin.c index 6ef8d5db..e7bff6dd 100644 --- a/tests/lib/plugin.c +++ b/tests/lib/plugin.c @@ -204,7 +204,6 @@ static void test_sfs(const char *plugin_dir) "the-sink", NULL, BT_LOGGING_LEVEL_NONE, &sink_component); ok(graph_ret == BT_GRAPH_ADD_COMPONENT_STATUS_OK && sink_component, "bt_graph_add_sink_component() still works after the plugin object is destroyed"); - BT_COMPONENT_SINK_PUT_REF_AND_RESET(sink_component); bt_graph_put_ref(graph); free(sfs_path); diff --git a/tests/lib/test_graph_topo.c b/tests/lib/test_graph_topo.c index d3d45818..d6c4c074 100644 --- a/tests/lib/test_graph_topo.c +++ b/tests/lib/test_graph_topo.c @@ -505,8 +505,6 @@ void test_src_adds_port_in_port_connected(void) ok(src_port_connected_pos < graph_port_added_src_pos, "event order is good"); - bt_component_source_put_ref(src); - bt_component_sink_put_ref(sink); bt_graph_put_ref(graph); } @@ -573,9 +571,7 @@ void test_simple(void) event.data.sink_comp_input_port_connected.other_port = gsrc_def_port; ok(has_event(&event), "got the expected sink's port connected event"); - bt_component_sink_put_ref(sink); bt_graph_put_ref(graph); - bt_component_source_put_ref(src); } static @@ -639,9 +635,6 @@ void test_src_port_connected_error(void) ok(has_event(&event), "got the expected source's port connected event"); bt_graph_put_ref(graph); - bt_component_sink_put_ref(sink); - bt_component_source_put_ref(src); - bt_connection_put_ref(conn); } static @@ -711,10 +704,7 @@ void test_sink_port_connected_error(void) event.data.sink_comp_input_port_connected.other_port = gsrc_def_port; ok(has_event(&event), "got the expected sink's port connected event"); - bt_connection_put_ref(conn); bt_graph_put_ref(graph); - bt_component_sink_put_ref(sink); - bt_component_source_put_ref(src); } static diff --git a/tests/lib/test_remove_destruction_listener_in_destruction_listener.c b/tests/lib/test_remove_destruction_listener_in_destruction_listener.c index 2cc1b76c..3e3d0037 100644 --- a/tests/lib/test_remove_destruction_listener_in_destruction_listener.c +++ b/tests/lib/test_remove_destruction_listener_in_destruction_listener.c @@ -278,7 +278,6 @@ int main(int argc, char **argv) BT_LOGGING_LEVEL_WARNING, &source); BT_ASSERT(add_component_status == BT_GRAPH_ADD_COMPONENT_STATUS_OK); - bt_component_source_put_ref(source); bt_component_class_source_put_ref(source_cc); bt_message_iterator_class_put_ref(msg_iter_cls); bt_graph_put_ref(graph); diff --git a/tests/lib/test_simple_sink.c b/tests/lib/test_simple_sink.c index b12ab35a..7c09cf67 100644 --- a/tests/lib/test_simple_sink.c +++ b/tests/lib/test_simple_sink.c @@ -109,7 +109,6 @@ bt_graph *create_graph_with_source(const bt_port_output **out_port) *out_port = bt_component_source_borrow_output_port_by_index_const( src_comp, 0); BT_ASSERT(*out_port); - bt_component_source_put_ref(src_comp); bt_component_class_source_put_ref(src_comp_cls); bt_message_iterator_class_put_ref(msg_iter_cls); return graph; @@ -163,7 +162,6 @@ void test_simple_expect_run_once_status( ok((run_once_status < 0) == (err != NULL), "Current thread error is set if bt_graph_run_once returned an error"); - bt_component_sink_put_ref(sink_comp); bt_graph_put_ref(graph); if (err) { bt_error_release(err); -- 2.34.1