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 <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/8164
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/12266
Tested-by: jenkins <jenkins@lttng.org>

diff --git a/src/plugins/ctf/fs-src/fs.cpp b/src/plugins/ctf/fs-src/fs.cpp
index 1161cb2ffe4f1c7465c68140060604e365c27144..db8be9962ee943034631db9cb4a9a5496ffdbf4e 100644 (file)
--- 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 f80368addc8d09f2c87aed35511a9f9eefff16c7..405bb145c6e8fb4792c73588f3cd660e7294f483 100644 (file)
--- 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<ctf_fs_component, ctf_fs_component_deleter>;
+
     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 c4bd65410f6b5792648ebb4ab56240b55ec44947..1e87697c9b1585de296d31fac27736d10b871ff0 100644 (file)
--- 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;
 }