From f340a3e89608bd8ca67e69e65ff593c714289253 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Mon, 11 Dec 2023 12:21:01 -0500 Subject: [PATCH] src.ctf.fs: use unique_ptr to manage ctf_fs_component lifetime Define ctf_fs_component::UP to be a unique_ptr with a deleter that calls ctf_fs_destroy. Change ctf_fs_component_create to return a ctf_fs_component::UP and adjust the appropriate callers / callees. Move where the ctf_fs_component instance is assigned as the "data" of the self component. Make it so it's assigned in ctf_fs_init (the component initialization entry point), as the very last step. This way, ctf_fs_create can return a unique_ptr, and there's no ambiguity on who owns the ctf_fs_component instance at any given time, and what happens in case of failure. Change-Id: I3ab1d70b0e7aa7772b7fa09deec70db7ea5a022c Signed-off-by: Simon Marchi Reviewed-on: https://review.lttng.org/c/babeltrace/+/8164 Reviewed-by: Philippe Proulx Reviewed-on: https://review.lttng.org/c/babeltrace/+/12266 Tested-by: jenkins --- src/plugins/ctf/fs-src/fs.cpp | 52 +++++++++++++++----------------- src/plugins/ctf/fs-src/fs.hpp | 9 +++++- src/plugins/ctf/fs-src/query.cpp | 11 ++----- 3 files changed, 35 insertions(+), 37 deletions(-) diff --git a/src/plugins/ctf/fs-src/fs.cpp b/src/plugins/ctf/fs-src/fs.cpp index 1161cb2f..db8be996 100644 --- a/src/plugins/ctf/fs-src/fs.cpp +++ b/src/plugins/ctf/fs-src/fs.cpp @@ -313,6 +313,11 @@ void ctf_fs_destroy(struct ctf_fs_component *ctf_fs) delete ctf_fs; } +void ctf_fs_component_deleter::operator()(ctf_fs_component *comp) +{ + ctf_fs_destroy(comp); +} + static void port_data_destroy(struct ctf_fs_port_data *port_data) { if (!port_data) { @@ -333,9 +338,10 @@ static void ctf_fs_trace_destroy_notifier(void *data) ctf_fs_trace_destroy(trace); } -ctf_fs_component *ctf_fs_component_create(const bt2c::Logger& parentLogger) +ctf_fs_component::UP ctf_fs_component_create(const bt2c::Logger& parentLogger) { - ctf_fs_component *ctf_fs = new ctf_fs_component {parentLogger}; + ctf_fs_component::UP ctf_fs {new ctf_fs_component {parentLogger}}; + ctf_fs->port_data = g_ptr_array_new_with_free_func(port_data_destroy_notifier); if (!ctf_fs->port_data) { goto error; @@ -344,8 +350,7 @@ ctf_fs_component *ctf_fs_component_create(const bt2c::Logger& parentLogger) goto end; error: - ctf_fs_destroy(ctf_fs); - ctf_fs = NULL; + ctf_fs.reset(); end: return ctf_fs; @@ -2190,46 +2195,36 @@ end: return ret; } -static struct ctf_fs_component *ctf_fs_create(const bt_value *params, - bt_self_component_source *self_comp_src) +static ctf_fs_component::UP ctf_fs_create(const bt_value *params, + bt_self_component_source *self_comp_src) { - struct ctf_fs_component *ctf_fs = NULL; const bt_value *inputs_value; const bt_value *trace_name_value; bt_self_component *self_comp = bt_self_component_source_as_self_component(self_comp_src); - ctf_fs = ctf_fs_component_create( + ctf_fs_component::UP ctf_fs = ctf_fs_component_create( bt2c::Logger {bt2::SelfSourceComponent {self_comp_src}, "PLUGIN/SRC.CTF.FS/COMP"}); if (!ctf_fs) { - goto error; + return nullptr; } - if (!read_src_fs_parameters(params, &inputs_value, &trace_name_value, ctf_fs)) { - goto error; + if (!read_src_fs_parameters(params, &inputs_value, &trace_name_value, ctf_fs.get())) { + return nullptr; } - bt_self_component_set_data(self_comp, ctf_fs); - - if (ctf_fs_component_create_ctf_fs_trace(ctf_fs, inputs_value, trace_name_value, self_comp)) { - goto error; + if (ctf_fs_component_create_ctf_fs_trace(ctf_fs.get(), inputs_value, trace_name_value, + self_comp)) { + return nullptr; } if (create_streams_for_trace(ctf_fs->trace)) { - goto error; + return nullptr; } - if (create_ports_for_trace(ctf_fs, ctf_fs->trace, self_comp_src)) { - goto error; + if (create_ports_for_trace(ctf_fs.get(), ctf_fs->trace, self_comp_src)) { + return nullptr; } - goto end; - -error: - ctf_fs_destroy(ctf_fs); - ctf_fs = NULL; - bt_self_component_set_data(self_comp, NULL); - -end: return ctf_fs; } @@ -2238,15 +2233,16 @@ bt_component_class_initialize_method_status ctf_fs_init(bt_self_component_source const bt_value *params, void *) { try { - struct ctf_fs_component *ctf_fs; bt_component_class_initialize_method_status ret = BT_COMPONENT_CLASS_INITIALIZE_METHOD_STATUS_OK; - ctf_fs = ctf_fs_create(params, self_comp_src); + ctf_fs_component::UP ctf_fs = ctf_fs_create(params, self_comp_src); if (!ctf_fs) { ret = BT_COMPONENT_CLASS_INITIALIZE_METHOD_STATUS_ERROR; } + bt_self_component_set_data(bt_self_component_source_as_self_component(self_comp_src), + ctf_fs.release()); return ret; } catch (const std::bad_alloc&) { return BT_COMPONENT_CLASS_INITIALIZE_METHOD_STATUS_MEMORY_ERROR; diff --git a/src/plugins/ctf/fs-src/fs.hpp b/src/plugins/ctf/fs-src/fs.hpp index f80368ad..405bb145 100644 --- a/src/plugins/ctf/fs-src/fs.hpp +++ b/src/plugins/ctf/fs-src/fs.hpp @@ -55,8 +55,15 @@ struct ctf_fs_metadata int bo = 0; }; +struct ctf_fs_component_deleter +{ + void operator()(struct ctf_fs_component *); +}; + struct ctf_fs_component { + using UP = std::unique_ptr; + explicit ctf_fs_component(const bt2c::Logger& parentLogger) : logger {parentLogger, "PLUGIN/SRC.CTF.FS/COMP"} { @@ -231,7 +238,7 @@ ctf_fs_iterator_seek_beginning(bt_self_message_iterator *message_iterator); /* Create and initialize a new, empty ctf_fs_component. */ -ctf_fs_component *ctf_fs_component_create(const bt2c::Logger& parentLogger); +ctf_fs_component::UP ctf_fs_component_create(const bt2c::Logger& parentLogger); /* * Create one `struct ctf_fs_trace` from one trace, or multiple traces sharing diff --git a/src/plugins/ctf/fs-src/query.cpp b/src/plugins/ctf/fs-src/query.cpp index c4bd6541..1e87697c 100644 --- a/src/plugins/ctf/fs-src/query.cpp +++ b/src/plugins/ctf/fs-src/query.cpp @@ -299,7 +299,7 @@ end: bt_component_class_query_method_status trace_infos_query(const bt_value *params, const bt2c::Logger& logger, const bt_value **user_result) { - struct ctf_fs_component *ctf_fs = NULL; + ctf_fs_component::UP ctf_fs; bt_component_class_query_method_status status = BT_COMPONENT_CLASS_QUERY_METHOD_STATUS_OK; bt_value *result = NULL; const bt_value *inputs_value = NULL; @@ -321,12 +321,12 @@ trace_infos_query(const bt_value *params, const bt2c::Logger& logger, const bt_v goto error; } - if (!read_src_fs_parameters(params, &inputs_value, &trace_name_value, ctf_fs)) { + if (!read_src_fs_parameters(params, &inputs_value, &trace_name_value, ctf_fs.get())) { status = BT_COMPONENT_CLASS_QUERY_METHOD_STATUS_ERROR; goto error; } - if (ctf_fs_component_create_ctf_fs_trace(ctf_fs, inputs_value, trace_name_value, NULL)) { + if (ctf_fs_component_create_ctf_fs_trace(ctf_fs.get(), inputs_value, trace_name_value, NULL)) { goto error; } @@ -357,11 +357,6 @@ error: } end: - if (ctf_fs) { - ctf_fs_destroy(ctf_fs); - ctf_fs = NULL; - } - *user_result = result; return status; } -- 2.34.1