From a256a42d2324f0ba50153f29e2f7ef513a9905c0 Mon Sep 17 00:00:00 2001 From: Philippe Proulx Date: Wed, 7 Jun 2017 12:38:17 -0400 Subject: [PATCH] Make bt_graph_connect_ports() return a status code MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit bt_graph_connect_ports() can fail for multiple reasons, some of which are not errors per se, but conditions which make the connection impossible, for example a component refuses the connection, or the graph is canceled. With this version, the connection is returned as an output parameter, which can be NULL if the user is not interested in the connection object (most common case), and the function returns a graph status code. Two new status codes are added in order to satisfy this patch: * BT_GRAPH_STATUS_COMPONENT_REFUSES_PORT_CONNECTION * BT_GRAPH_STATUS_NOMEM The first one is not considered an error, that is, the graph's user could only _try_ to connect two components until the function returns BT_GRAPH_STATUS_OK (and retry in case of BT_GRAPH_STATUS_COMPONENT_REFUSES_PORT_CONNECTION). Signed-off-by: Philippe Proulx Signed-off-by: Jérémie Galarneau --- cli/babeltrace.c | 36 +++++++++++++++--- include/babeltrace/graph/graph-internal.h | 37 ++++++++++++++++++ include/babeltrace/graph/graph.h | 8 ++-- lib/graph/graph.c | 46 ++++++++++++++++------- tests/lib/test_bt_notification_iterator.c | 6 +-- tests/lib/test_graph_topo.c | 23 ++++++++---- tests/plugins/test-utils-muxer.c | 36 ++++++++---------- 7 files changed, 138 insertions(+), 54 deletions(-) diff --git a/cli/babeltrace.c b/cli/babeltrace.c index 66c74df4..b68ddf15 100644 --- a/cli/babeltrace.c +++ b/cli/babeltrace.c @@ -1178,7 +1178,7 @@ int cmd_run_ctx_connect_upstream_port_to_downstream_component( uint64_t i; int64_t (*port_count_fn)(struct bt_component *); struct bt_port *(*port_by_index_fn)(struct bt_component *, uint64_t); - void *conn = NULL; + enum bt_graph_status status = BT_GRAPH_STATUS_ERROR; BT_LOGI("Connecting upstream port to the next available downstream port: " "upstream-port-addr=%p, upstream-port-name=\"%s\", " @@ -1243,10 +1243,35 @@ int cmd_run_ctx_connect_upstream_port_to_downstream_component( cfg_conn->downstream_port_glob->str, -1ULL, downstream_port_name, -1ULL)) { /* We have a winner! */ - conn = bt_graph_connect_ports(ctx->graph, - upstream_port, downstream_port); + status = bt_graph_connect_ports(ctx->graph, + upstream_port, downstream_port, NULL); bt_put(downstream_port); - if (!conn) { + switch (status) { + case BT_GRAPH_STATUS_OK: + break; + case BT_GRAPH_STATUS_CANCELED: + BT_LOGI_STR("Graph was canceled by user."); + status = BT_GRAPH_STATUS_OK; + break; + case BT_GRAPH_STATUS_COMPONENT_REFUSES_PORT_CONNECTION: + BT_LOGE("A component refused a connection to one of its ports: " + "upstream-comp-addr=%p, upstream-comp-name=\"%s\", " + "upstream-port-addr=%p, upstream-port-name=\"%s\", " + "downstream-comp-addr=%p, downstream-comp-name=\"%s\", " + "downstream-port-addr=%p, downstream-port-name=\"%s\", " + "conn-arg=\"%s\"", + upstream_comp, bt_component_get_name(upstream_comp), + upstream_port, bt_port_get_name(upstream_port), + downstream_comp, cfg_conn->downstream_comp_name->str, + downstream_port, downstream_port_name, + cfg_conn->arg->str); + fprintf(stderr, + "A component refused a connection to one of its ports (`%s` to `%s`): %s\n", + bt_port_get_name(upstream_port), + downstream_port_name, + cfg_conn->arg->str); + break; + default: BT_LOGE("Cannot create connection: graph refuses to connect ports: " "upstream-comp-addr=%p, upstream-comp-name=\"%s\", " "upstream-port-addr=%p, upstream-port-name=\"%s\", " @@ -1284,7 +1309,7 @@ int cmd_run_ctx_connect_upstream_port_to_downstream_component( bt_put(downstream_port); } - if (!conn) { + if (status != BT_GRAPH_STATUS_OK) { BT_LOGE("Cannot create connection: cannot find a matching downstream port for upstream port: " "upstream-port-addr=%p, upstream-port-name=\"%s\", " "downstream-comp-name=\"%s\", conn-arg=\"%s\"", @@ -1303,7 +1328,6 @@ error: ret = -1; end: - bt_put(conn); return ret; } diff --git a/include/babeltrace/graph/graph-internal.h b/include/babeltrace/graph/graph-internal.h index 7e8ea45f..f8f0d1a1 100644 --- a/include/babeltrace/graph/graph-internal.h +++ b/include/babeltrace/graph/graph-internal.h @@ -28,8 +28,10 @@ */ #include +#include #include #include +#include #include struct bt_component; @@ -107,9 +109,44 @@ const char *bt_graph_status_string(enum bt_graph_status status) return "BT_GRAPH_STATUS_NO_SINK"; case BT_GRAPH_STATUS_ERROR: return "BT_GRAPH_STATUS_ERROR"; + case BT_GRAPH_STATUS_COMPONENT_REFUSES_PORT_CONNECTION: + return "BT_GRAPH_STATUS_COMPONENT_REFUSES_PORT_CONNECTION"; + case BT_GRAPH_STATUS_NOMEM: + return "BT_GRAPH_STATUS_NOMEM"; default: return "(unknown)"; } } +static inline +enum bt_graph_status bt_graph_status_from_component_status( + enum bt_component_status comp_status) +{ + switch (comp_status) { + case BT_COMPONENT_STATUS_OK: + return BT_GRAPH_STATUS_OK; + case BT_COMPONENT_STATUS_END: + return BT_GRAPH_STATUS_END; + case BT_COMPONENT_STATUS_AGAIN: + return BT_GRAPH_STATUS_AGAIN; + case BT_COMPONENT_STATUS_REFUSE_PORT_CONNECTION: + return BT_GRAPH_STATUS_COMPONENT_REFUSES_PORT_CONNECTION; + case BT_COMPONENT_STATUS_ERROR: + return BT_GRAPH_STATUS_ERROR; + case BT_COMPONENT_STATUS_UNSUPPORTED: + return BT_GRAPH_STATUS_ERROR; + case BT_COMPONENT_STATUS_INVALID: + return BT_GRAPH_STATUS_INVALID; + case BT_COMPONENT_STATUS_NOMEM: + return BT_GRAPH_STATUS_NOMEM; + case BT_COMPONENT_STATUS_NOT_FOUND: + return BT_GRAPH_STATUS_ERROR; + default: +#ifdef BT_LOGF + BT_LOGF("Unknown component status: status=%d", comp_status); +#endif + abort(); + } +} + #endif /* BABELTRACE_COMPONENT_COMPONENT_GRAPH_INTERNAL_H */ diff --git a/include/babeltrace/graph/graph.h b/include/babeltrace/graph/graph.h index 06f2af1b..a2dfbd82 100644 --- a/include/babeltrace/graph/graph.h +++ b/include/babeltrace/graph/graph.h @@ -38,6 +38,7 @@ struct bt_port; struct bt_connection; enum bt_graph_status { + BT_GRAPH_STATUS_COMPONENT_REFUSES_PORT_CONNECTION = 111, /** Canceled. */ BT_GRAPH_STATUS_CANCELED = 125, /** No sink can consume at the moment. */ @@ -53,6 +54,7 @@ enum bt_graph_status { BT_GRAPH_STATUS_NO_SINK = -6, /** General error. */ BT_GRAPH_STATUS_ERROR = -1, + BT_GRAPH_STATUS_NOMEM = -12, }; typedef void (*bt_graph_port_added_listener)(struct bt_port *port, @@ -73,9 +75,9 @@ extern struct bt_graph *bt_graph_create(void); * Creates a connection between two components using the two ports specified * and adds the connection and components (if not already added) to the graph. */ -extern struct bt_connection *bt_graph_connect_ports(struct bt_graph *graph, - struct bt_port *upstream, - struct bt_port *downstream); +extern enum bt_graph_status bt_graph_connect_ports(struct bt_graph *graph, + struct bt_port *upstream, struct bt_port *downstream, + struct bt_connection **connection); /** * Run graph to completion or until a single sink is left and "AGAIN" is received. diff --git a/lib/graph/graph.c b/lib/graph/graph.c index e71eaeed..c66b4f3c 100644 --- a/lib/graph/graph.c +++ b/lib/graph/graph.c @@ -191,10 +191,11 @@ error: goto end; } -struct bt_connection *bt_graph_connect_ports(struct bt_graph *graph, - struct bt_port *upstream_port, - struct bt_port *downstream_port) +enum bt_graph_status bt_graph_connect_ports(struct bt_graph *graph, + struct bt_port *upstream_port, struct bt_port *downstream_port, + struct bt_connection **user_connection) { + enum bt_graph_status status = BT_GRAPH_STATUS_OK; struct bt_connection *connection = NULL; struct bt_graph *upstream_graph = NULL; struct bt_graph *downstream_graph = NULL; @@ -206,16 +207,19 @@ struct bt_connection *bt_graph_connect_ports(struct bt_graph *graph, if (!graph) { BT_LOGW_STR("Invalid parameter: graph is NULL."); + status = BT_GRAPH_STATUS_INVALID; goto end; } if (!upstream_port) { BT_LOGW_STR("Invalid parameter: upstream port is NULL."); + status = BT_GRAPH_STATUS_INVALID; goto end; } if (!downstream_port) { BT_LOGW_STR("Invalid parameter: downstream port is NULL."); + status = BT_GRAPH_STATUS_INVALID; goto end; } @@ -228,27 +232,32 @@ struct bt_connection *bt_graph_connect_ports(struct bt_graph *graph, if (graph->canceled) { BT_LOGW_STR("Invalid parameter: graph is canceled."); + status = BT_GRAPH_STATUS_CANCELED; goto end; } /* Ensure appropriate types for upstream and downstream ports. */ if (bt_port_get_type(upstream_port) != BT_PORT_TYPE_OUTPUT) { BT_LOGW_STR("Invalid parameter: upstream port is not an output port."); + status = BT_GRAPH_STATUS_INVALID; goto end; } if (bt_port_get_type(downstream_port) != BT_PORT_TYPE_INPUT) { BT_LOGW_STR("Invalid parameter: downstream port is not an input port."); + status = BT_GRAPH_STATUS_INVALID; goto end; } /* Ensure that both ports are currently unconnected. */ if (bt_port_is_connected(upstream_port)) { BT_LOGW_STR("Invalid parameter: upstream port is already connected."); + status = BT_GRAPH_STATUS_INVALID; goto end; } if (bt_port_is_connected(downstream_port)) { BT_LOGW_STR("Invalid parameter: downstream port is already connected."); + status = BT_GRAPH_STATUS_INVALID; goto end; } @@ -259,12 +268,14 @@ struct bt_connection *bt_graph_connect_ports(struct bt_graph *graph, upstream_component = bt_port_get_component(upstream_port); if (!upstream_component) { BT_LOGW_STR("Invalid parameter: upstream port is loose (does not belong to a component)"); + status = BT_GRAPH_STATUS_INVALID; goto end; } downstream_component = bt_port_get_component(downstream_port); if (!downstream_component) { BT_LOGW_STR("Invalid parameter: downstream port is loose (does not belong to a component)"); + status = BT_GRAPH_STATUS_INVALID; goto end; } @@ -279,14 +290,16 @@ struct bt_connection *bt_graph_connect_ports(struct bt_graph *graph, if (upstream_graph && (graph != upstream_graph)) { BT_LOGW("Invalid parameter: upstream port's component is already part of another graph: " "other-graph-addr=%p", upstream_graph); - goto error; + status = BT_GRAPH_STATUS_ALREADY_IN_A_GRAPH; + goto end; } upstream_was_already_in_graph = (graph == upstream_graph); downstream_graph = bt_component_get_graph(downstream_component); if (downstream_graph && (graph != downstream_graph)) { BT_LOGW("Invalid parameter: downstream port's component is already part of another graph: " "other-graph-addr=%p", downstream_graph); - goto error; + status = BT_GRAPH_STATUS_ALREADY_IN_A_GRAPH; + goto end; } downstream_was_already_in_graph = (graph == downstream_graph); @@ -306,7 +319,9 @@ struct bt_connection *bt_graph_connect_ports(struct bt_graph *graph, "status=%s", bt_component_status_string(component_status)); } - goto error; + status = bt_graph_status_from_component_status( + component_status); + goto end; } BT_LOGD_STR("Asking downstream component to accept the connection."); @@ -320,7 +335,9 @@ struct bt_connection *bt_graph_connect_ports(struct bt_graph *graph, "status=%s", bt_component_status_string(component_status)); } - goto error; + status = bt_graph_status_from_component_status( + component_status); + goto end; } BT_LOGD_STR("Creating connection."); @@ -328,7 +345,8 @@ struct bt_connection *bt_graph_connect_ports(struct bt_graph *graph, downstream_port); if (!connection) { BT_LOGW("Cannot create connection object."); - goto error; + status = BT_GRAPH_STATUS_NOMEM; + goto end; } BT_LOGD("Connection object created: conn-addr=%p", connection); @@ -386,17 +404,17 @@ struct bt_connection *bt_graph_connect_ports(struct bt_graph *graph, upstream_port, bt_port_get_name(upstream_port), downstream_port, bt_port_get_name(downstream_port)); + if (user_connection) { + BT_MOVE(*user_connection, connection); + } + end: bt_put(upstream_graph); bt_put(downstream_graph); bt_put(upstream_component); bt_put(downstream_component); - return connection; - -error: - BT_PUT(upstream_component); - BT_PUT(downstream_component); - goto end; + bt_put(connection); + return status; } enum bt_graph_status bt_graph_consume(struct bt_graph *graph) diff --git a/tests/lib/test_bt_notification_iterator.c b/tests/lib/test_bt_notification_iterator.c index 6ba99952..cace4019 100644 --- a/tests/lib/test_bt_notification_iterator.c +++ b/tests/lib/test_bt_notification_iterator.c @@ -917,7 +917,6 @@ void do_std_test(enum test test, const char *name, struct bt_port *upstream_port; struct bt_port *downstream_port; struct bt_graph *graph; - void *conn; enum bt_graph_status graph_status = BT_GRAPH_STATUS_OK; clear_test_events(); @@ -932,9 +931,8 @@ void do_std_test(enum test test, const char *name, assert(upstream_port); downstream_port = bt_component_sink_get_input_port_by_name(sink_comp, "in"); assert(downstream_port); - conn = bt_graph_connect_ports(graph, upstream_port, downstream_port); - assert(conn); - bt_put(conn); + graph_status = bt_graph_connect_ports(graph, upstream_port, + downstream_port, NULL); bt_put(upstream_port); bt_put(downstream_port); diff --git a/tests/lib/test_graph_topo.c b/tests/lib/test_graph_topo.c index 92ebac80..6b1976d1 100644 --- a/tests/lib/test_graph_topo.c +++ b/tests/lib/test_graph_topo.c @@ -650,6 +650,7 @@ void test_sink_removes_port_in_port_connected_then_src_removes_disconnected_port struct bt_port *sink_def_port; struct bt_connection *conn; struct event event; + enum bt_graph_status status; size_t src_accept_port_connection_pos; size_t sink_accept_port_connection_pos; size_t src_port_connected_pos; @@ -670,7 +671,9 @@ void test_sink_removes_port_in_port_connected_then_src_removes_disconnected_port assert(src_def_port); sink_def_port = bt_component_sink_get_input_port_by_name(sink, "in"); assert(sink_def_port); - conn = bt_graph_connect_ports(graph, src_def_port, sink_def_port); + status = bt_graph_connect_ports(graph, src_def_port, sink_def_port, + &conn); + assert(status == 0); assert(conn); /* We're supposed to have 5 events so far */ @@ -812,6 +815,7 @@ void test_sink_removes_port_in_port_connected(void) struct bt_port *sink_def_port; struct bt_connection *conn; struct event event; + enum bt_graph_status status; size_t src_accept_port_connection_pos; size_t sink_accept_port_connection_pos; size_t src_port_connected_pos; @@ -831,8 +835,9 @@ void test_sink_removes_port_in_port_connected(void) assert(src_def_port); sink_def_port = bt_component_sink_get_input_port_by_name(sink, "in"); assert(sink_def_port); - conn = bt_graph_connect_ports(graph, src_def_port, sink_def_port); - assert(conn); + status = bt_graph_connect_ports(graph, src_def_port, sink_def_port, + &conn); + assert(status == 0); /* We're supposed to have 5 events so far */ ok(events->len == 5, "we have the expected number of events (before consume)"); @@ -958,6 +963,7 @@ void test_src_adds_port_in_port_connected(void) struct bt_port *src_hello_port; struct bt_connection *conn; struct event event; + enum bt_graph_status status; size_t src_accept_port_connection_pos; size_t sink_accept_port_connection_pos; size_t src_port_connected_pos; @@ -974,8 +980,9 @@ void test_src_adds_port_in_port_connected(void) assert(src_def_port); sink_def_port = bt_component_sink_get_input_port_by_name(sink, "in"); assert(sink_def_port); - conn = bt_graph_connect_ports(graph, src_def_port, sink_def_port); - assert(conn); + status = bt_graph_connect_ports(graph, src_def_port, sink_def_port, + &conn); + assert(status == 0); src_hello_port = bt_component_source_get_output_port_by_name(src, "hello"); assert(src_hello_port); @@ -1065,6 +1072,7 @@ void test_simple(void) struct bt_port *sink_def_port; struct bt_connection *conn; struct event event; + enum bt_graph_status status; size_t src_accept_port_connection_pos; size_t sink_accept_port_connection_pos; size_t src_port_connected_pos; @@ -1079,8 +1087,9 @@ void test_simple(void) assert(src_def_port); sink_def_port = bt_component_sink_get_input_port_by_name(sink, "in"); assert(sink_def_port); - conn = bt_graph_connect_ports(graph, src_def_port, sink_def_port); - assert(conn); + status = bt_graph_connect_ports(graph, src_def_port, sink_def_port, + &conn); + assert(status == 0); /* We're supposed to have 5 events */ ok(events->len == 5, "we have the expected number of events"); diff --git a/tests/plugins/test-utils-muxer.c b/tests/plugins/test-utils-muxer.c index a706b99b..e9d2dca0 100644 --- a/tests/plugins/test-utils-muxer.c +++ b/tests/plugins/test-utils-muxer.c @@ -988,7 +988,6 @@ void do_std_test(enum test test, const char *name, struct bt_graph *graph; int64_t i; int64_t count; - void *conn; enum bt_graph_status graph_status = BT_GRAPH_STATUS_OK; clear_test_events(); @@ -1010,10 +1009,9 @@ void do_std_test(enum test test, const char *name, downstream_port = bt_component_filter_get_input_port_by_index( muxer_comp, i); assert(downstream_port); - conn = bt_graph_connect_ports(graph, - upstream_port, downstream_port); - assert(conn); - bt_put(conn); + graph_status = bt_graph_connect_ports(graph, + upstream_port, downstream_port, NULL); + assert(graph_status == 0); bt_put(upstream_port); bt_put(downstream_port); } @@ -1025,9 +1023,9 @@ void do_std_test(enum test test, const char *name, assert(upstream_port); downstream_port = bt_component_sink_get_input_port_by_name(sink_comp, "in"); assert(downstream_port); - conn = bt_graph_connect_ports(graph, upstream_port, downstream_port); - assert(conn); - bt_put(conn); + graph_status = bt_graph_connect_ports(graph, upstream_port, + downstream_port, NULL); + assert(graph_status == 0); bt_put(upstream_port); bt_put(downstream_port); @@ -1345,9 +1343,9 @@ void connect_port_to_first_avail_muxer_port(struct bt_graph *graph, struct bt_component *muxer_comp) { struct bt_port *avail_muxer_port = NULL; - void *conn; int64_t i; int64_t count; + enum bt_graph_status graph_status; count = bt_component_filter_get_input_port_count(muxer_comp); assert(count >= 0); @@ -1367,9 +1365,9 @@ void connect_port_to_first_avail_muxer_port(struct bt_graph *graph, } } - conn = bt_graph_connect_ports(graph, source_port, avail_muxer_port); - assert(conn); - bt_put(conn); + graph_status = bt_graph_connect_ports(graph, source_port, + avail_muxer_port, NULL); + assert(graph_status == 0); bt_put(avail_muxer_port); } @@ -1405,7 +1403,6 @@ void test_single_end_then_multiple_full(void) struct bt_graph *graph; int64_t i; int64_t count; - void *conn; int ret; enum bt_graph_status graph_status = BT_GRAPH_STATUS_OK; struct graph_listener_data graph_listener_data; @@ -1501,9 +1498,9 @@ void test_single_end_then_multiple_full(void) assert(upstream_port); downstream_port = bt_component_sink_get_input_port_by_name(sink_comp, "in"); assert(downstream_port); - conn = bt_graph_connect_ports(graph, upstream_port, downstream_port); - assert(conn); - bt_put(conn); + graph_status = bt_graph_connect_ports(graph, upstream_port, + downstream_port, NULL); + assert(graph_status == 0); bt_put(upstream_port); bt_put(downstream_port); @@ -1533,7 +1530,6 @@ void test_single_again_end_then_multiple_full(void) struct bt_graph *graph; int64_t i; int64_t count; - void *conn; int ret; enum bt_graph_status graph_status = BT_GRAPH_STATUS_OK; struct graph_listener_data graph_listener_data; @@ -1630,9 +1626,9 @@ void test_single_again_end_then_multiple_full(void) assert(upstream_port); downstream_port = bt_component_sink_get_input_port_by_name(sink_comp, "in"); assert(downstream_port); - conn = bt_graph_connect_ports(graph, upstream_port, downstream_port); - assert(conn); - bt_put(conn); + graph_status = bt_graph_connect_ports(graph, upstream_port, + downstream_port, NULL); + assert(graph_status == 0); bt_put(upstream_port); bt_put(downstream_port); -- 2.34.1