ctf: use unique_ptr to manage ctf_metadata_decoder lifetime

Introduce the ctf_metadata_decoder_up type, a unique_ptr with a deleter
that calls ctf_metadata_decoder_destroy.  Change
ctf_metadata_decoder_create to return a ctf_metadata_decoder_up, and
adjust callers / callees accordingly.

Note that this is temporary, ctf_metadata_decoder is going to be deleted
in a future refactor.  But in the mean time, it helps make the callers
use RAII and become more exception-safe.

Change-Id: Ia0e24b425c47b90dc71b10d1c7fd0a3000c89180
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/8167
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/12270
Tested-by: jenkins <jenkins@lttng.org>

diff --git a/src/plugins/ctf/common/src/metadata/tsdl/decoder.cpp b/src/plugins/ctf/common/src/metadata/tsdl/decoder.cpp
index 739c648b278a10937f65c26741c2caebc6adf11a..b6fe517487f3a297ff87987a4a2052d265735da0 100644 (file)
--- a/src/plugins/ctf/common/src/metadata/tsdl/decoder.cpp
+++ b/src/plugins/ctf/common/src/metadata/tsdl/decoder.cpp
@@ -86,7 +86,7 @@ end:
     return ret;
 }
 
-struct ctf_metadata_decoder *
+ctf_metadata_decoder_up
 ctf_metadata_decoder_create(const struct ctf_metadata_decoder_config *config)
 {
     BT_ASSERT(config);
@@ -139,7 +139,7 @@ error:
     mdec = NULL;
 
 end:
-    return mdec;
+    return ctf_metadata_decoder_up {mdec};
 }
 
 void ctf_metadata_decoder_destroy(struct ctf_metadata_decoder *mdec)
@@ -161,6 +161,11 @@ void ctf_metadata_decoder_destroy(struct ctf_metadata_decoder *mdec)
     delete mdec;
 }
 
+void ctf_metadata_decoder_deleter::operator()(ctf_metadata_decoder *decoder)
+{
+    ctf_metadata_decoder_destroy(decoder);
+}
+
 enum ctf_metadata_decoder_status
 ctf_metadata_decoder_append_content(struct ctf_metadata_decoder *mdec, FILE *fp)
 {

diff --git a/src/plugins/ctf/common/src/metadata/tsdl/decoder.hpp b/src/plugins/ctf/common/src/metadata/tsdl/decoder.hpp
index 4460acaedd0617a44de82c1dc46be1a15f7d320d..0aee6173c845374cf6f10a390c5759bba2f42431 100644 (file)
--- a/src/plugins/ctf/common/src/metadata/tsdl/decoder.hpp
+++ b/src/plugins/ctf/common/src/metadata/tsdl/decoder.hpp
@@ -80,12 +80,19 @@ struct ctf_metadata_decoder_config
     bool keep_plain_text = false;
 };
 
+struct ctf_metadata_decoder_deleter
+{
+    void operator()(struct ctf_metadata_decoder *decoder);
+};
+
+using ctf_metadata_decoder_up = std::unique_ptr<ctf_metadata_decoder, ctf_metadata_decoder_deleter>;
+
 /*
  * Creates a CTF metadata decoder.
  *
  * Returns `NULL` on error.
  */
-struct ctf_metadata_decoder *
+ctf_metadata_decoder_up
 ctf_metadata_decoder_create(const struct ctf_metadata_decoder_config *config);
 
 /*

diff --git a/src/plugins/ctf/fs-src/fs.hpp b/src/plugins/ctf/fs-src/fs.hpp
index 54a8ec0234df248c846f992f111c684c8c0d9cb9..1cb9e8b058fef34c9bcf286300055506e26cdb30 100644 (file)
--- a/src/plugins/ctf/fs-src/fs.hpp
+++ b/src/plugins/ctf/fs-src/fs.hpp
@@ -18,6 +18,7 @@
 #include "cpp-common/bt2c/logging.hpp"
 
 #include "metadata.hpp"
+#include "plugins/ctf/common/src/metadata/tsdl/decoder.hpp"
 
 extern bool ctf_fs_debug;
 
@@ -42,7 +43,7 @@ struct ctf_fs_file
 struct ctf_fs_metadata
 {
     /* Owned by this */
-    struct ctf_metadata_decoder *decoder = nullptr;
+    ctf_metadata_decoder_up decoder;
 
     /* Owned by this */
     bt_trace_class *trace_class = nullptr;

diff --git a/src/plugins/ctf/fs-src/metadata.cpp b/src/plugins/ctf/fs-src/metadata.cpp
index cebd21fc5b3ccea194b343bf93f17352c056625a..cbe4172b21a6fab4ebb4f34c73e3f3781b9e5a31 100644 (file)
--- a/src/plugins/ctf/fs-src/metadata.cpp
+++ b/src/plugins/ctf/fs-src/metadata.cpp
@@ -90,17 +90,17 @@ int ctf_fs_metadata_set_trace_class(bt_self_component *self_comp, struct ctf_fs_
         goto end;
     }
 
-    ret = ctf_metadata_decoder_append_content(ctf_fs_trace->metadata->decoder, file->fp);
+    ret = ctf_metadata_decoder_append_content(ctf_fs_trace->metadata->decoder.get(), file->fp);
     if (ret) {
         BT_CPPLOGE_SPEC(ctf_fs_trace->logger, "Cannot update metadata decoder's content.");
         goto end;
     }
 
     ctf_fs_trace->metadata->trace_class =
-        ctf_metadata_decoder_get_ir_trace_class(ctf_fs_trace->metadata->decoder);
+        ctf_metadata_decoder_get_ir_trace_class(ctf_fs_trace->metadata->decoder.get());
     BT_ASSERT(!self_comp || ctf_fs_trace->metadata->trace_class);
     ctf_fs_trace->metadata->tc =
-        ctf_metadata_decoder_borrow_ctf_trace_class(ctf_fs_trace->metadata->decoder);
+        ctf_metadata_decoder_borrow_ctf_trace_class(ctf_fs_trace->metadata->decoder.get());
     BT_ASSERT(ctf_fs_trace->metadata->tc);
 
 end:
@@ -122,7 +122,5 @@ void ctf_fs_metadata_fini(struct ctf_fs_metadata *metadata)
         BT_TRACE_CLASS_PUT_REF_AND_RESET(metadata->trace_class);
     }
 
-    if (metadata->decoder) {
-        ctf_metadata_decoder_destroy(metadata->decoder);
-    }
+    metadata->decoder.reset();
 }

diff --git a/src/plugins/ctf/fs-src/query.cpp b/src/plugins/ctf/fs-src/query.cpp
index 95453ada5595eb4db7a10228bfcf95bc6135b10f..6625ec7d403e7fdbd1bd6508b253d1ff47d76372 100644 (file)
--- a/src/plugins/ctf/fs-src/query.cpp
+++ b/src/plugins/ctf/fs-src/query.cpp
@@ -39,7 +39,7 @@ bt_component_class_query_method_status metadata_info_query(const bt_value *param
     int bo;
     const char *path;
     bool is_packetized;
-    struct ctf_metadata_decoder *decoder = NULL;
+    ctf_metadata_decoder_up decoder;
     ctf_metadata_decoder_config decoder_cfg {logger};
     enum ctf_metadata_decoder_status decoder_status;
     GString *g_metadata_text = NULL;
@@ -97,14 +97,14 @@ bt_component_class_query_method_status metadata_info_query(const bt_value *param
     }
 
     rewind(metadata_fp.get());
-    decoder_status = ctf_metadata_decoder_append_content(decoder, metadata_fp.get());
+    decoder_status = ctf_metadata_decoder_append_content(decoder.get(), metadata_fp.get());
     if (decoder_status) {
         BT_CPPLOGE_APPEND_CAUSE_SPEC(
             logger, "Cannot update metadata decoder's content: path=\"{}\".", path);
         goto error;
     }
 
-    plaintext = ctf_metadata_decoder_get_text(decoder);
+    plaintext = ctf_metadata_decoder_get_text(decoder.get());
     g_metadata_text = g_string_new(NULL);
 
     if (!g_metadata_text) {
@@ -117,7 +117,6 @@ bt_component_class_query_method_status metadata_info_query(const bt_value *param
     }
 
     g_string_append(g_metadata_text, plaintext);
-
     ret = bt_value_map_insert_string_entry(result, "text", g_metadata_text->str);
     if (ret) {
         BT_CPPLOGE_APPEND_CAUSE_SPEC(logger, "Cannot insert metadata text into query result.");
@@ -145,7 +144,6 @@ end:
     if (g_metadata_text) {
         g_string_free(g_metadata_text, TRUE);
     }
-    ctf_metadata_decoder_destroy(decoder);
 
     *user_result = result;
     return status;
@@ -364,7 +362,7 @@ support_info_query(const bt_value *params, const bt2c::Logger& logger, const bt_
     double weight = 0;
     bt2c::GCharUP metadata_path;
     bt_value *result = NULL;
-    struct ctf_metadata_decoder *metadata_decoder = NULL;
+    ctf_metadata_decoder_up metadata_decoder;
     FILE *metadata_file = NULL;
     char uuid_str[BT_UUID_STR_LEN + 1];
     bool has_uuid = false;
@@ -404,7 +402,7 @@ support_info_query(const bt_value *params, const bt2c::Logger& logger, const bt_
             goto end;
         }
 
-        decoder_status = ctf_metadata_decoder_append_content(metadata_decoder, metadata_file);
+        decoder_status = ctf_metadata_decoder_append_content(metadata_decoder.get(), metadata_file);
         if (decoder_status != CTF_METADATA_DECODER_STATUS_OK) {
             BT_CPPLOGW_SPEC(logger, "cannot append metadata content: metadata-decoder-status={}",
                             decoder_status);
@@ -419,7 +417,7 @@ support_info_query(const bt_value *params, const bt2c::Logger& logger, const bt_
         weight = 0.75;
 
         /* If the trace has a UUID, return the stringified UUID as the group. */
-        if (ctf_metadata_decoder_get_trace_class_uuid(metadata_decoder, uuid) == 0) {
+        if (ctf_metadata_decoder_get_trace_class_uuid(metadata_decoder.get(), uuid) == 0) {
             bt_uuid_to_str(uuid, uuid_str);
             has_uuid = true;
         }
@@ -455,7 +453,6 @@ create_result:
 
 end:
     bt_value_put_ref(result);
-    ctf_metadata_decoder_destroy(metadata_decoder);
 
     return status;
 }

diff --git a/src/plugins/ctf/lttng-live/data-stream.cpp b/src/plugins/ctf/lttng-live/data-stream.cpp
index c83d84915c9086e32d9ea3fe9e54af20c9f574b4..5ba068c379eb1c372ff5382aa844d9916fe82f78 100644 (file)
--- a/src/plugins/ctf/lttng-live/data-stream.cpp
+++ b/src/plugins/ctf/lttng-live/data-stream.cpp
@@ -136,7 +136,7 @@ enum lttng_live_iterator_status lttng_live_lazy_msg_init(struct lttng_live_sessi
             if (stream_iter->msg_iter) {
                 continue;
             }
-            ctf_tc = ctf_metadata_decoder_borrow_ctf_trace_class(trace->metadata->decoder);
+            ctf_tc = ctf_metadata_decoder_borrow_ctf_trace_class(trace->metadata->decoder.get());
             BT_CPPLOGD_SPEC(stream_iter->logger,
                             "Creating CTF message iterator: session-id={}, ctf-tc-addr={}, "
                             "stream-iter-name={}, self-msg-iter-addr={}",
@@ -192,7 +192,7 @@ lttng_live_stream_iterator_create(struct lttng_live_session *session, uint64_t c
 
     if (trace->trace) {
         struct ctf_trace_class *ctf_tc =
-            ctf_metadata_decoder_borrow_ctf_trace_class(trace->metadata->decoder);
+            ctf_metadata_decoder_borrow_ctf_trace_class(trace->metadata->decoder.get());
         BT_ASSERT(!stream_iter->msg_iter);
         stream_iter->msg_iter =
             ctf_msg_iter_create(ctf_tc, lttng_live->max_query_size, medops, stream_iter,

diff --git a/src/plugins/ctf/lttng-live/lttng-live.hpp b/src/plugins/ctf/lttng-live/lttng-live.hpp
index 7d93326f6e31c9b35e417615d4a381325ccbda30..8016efc258e9052f50745d8feb654492ae978a42 100644 (file)
--- a/src/plugins/ctf/lttng-live/lttng-live.hpp
+++ b/src/plugins/ctf/lttng-live/lttng-live.hpp
@@ -148,8 +148,9 @@ struct lttng_live_metadata
     bt2c::Logger logger;
 
     uint64_t stream_id = 0;
+
     /* Weak reference. */
-    struct ctf_metadata_decoder *decoder = nullptr;
+    ctf_metadata_decoder_up decoder;
 };
 
 enum lttng_live_metadata_stream_state

diff --git a/src/plugins/ctf/lttng-live/metadata.cpp b/src/plugins/ctf/lttng-live/metadata.cpp
index 6aaa7beb68f8037ff463ecad3831e4ebbe891cb1..e90b352ee46c1f56e81fb01d8b730586fa5f935c 100644 (file)
--- a/src/plugins/ctf/lttng-live/metadata.cpp
+++ b/src/plugins/ctf/lttng-live/metadata.cpp
@@ -226,14 +226,14 @@ enum lttng_live_iterator_status lttng_live_metadata_update(struct lttng_live_tra
      * new metadata to our current trace class.
      */
     BT_CPPLOGD_SPEC(metadata->logger, "Appending new metadata to the ctf_trace class");
-    decoder_status = ctf_metadata_decoder_append_content(metadata->decoder, fp);
+    decoder_status = ctf_metadata_decoder_append_content(metadata->decoder.get(), fp);
     switch (decoder_status) {
     case CTF_METADATA_DECODER_STATUS_OK:
         if (!trace->trace_class) {
             struct ctf_trace_class *tc =
-                ctf_metadata_decoder_borrow_ctf_trace_class(metadata->decoder);
+                ctf_metadata_decoder_borrow_ctf_trace_class(metadata->decoder.get());
 
-            trace->trace_class = ctf_metadata_decoder_get_ir_trace_class(metadata->decoder);
+            trace->trace_class = ctf_metadata_decoder_get_ir_trace_class(metadata->decoder.get());
             trace->trace = bt_trace_create(trace->trace_class);
             if (!trace->trace) {
                 BT_CPPLOGE_APPEND_CAUSE_SPEC(metadata->logger, "Failed to create bt_trace");
@@ -305,7 +305,6 @@ int lttng_live_metadata_create_stream(struct lttng_live_session *session, uint64
     return 0;
 
 error:
-    ctf_metadata_decoder_destroy(metadata->decoder);
     delete metadata;
     return -1;
 }
@@ -317,7 +316,6 @@ void lttng_live_metadata_fini(struct lttng_live_trace *trace)
     if (!metadata) {
         return;
     }
-    ctf_metadata_decoder_destroy(metadata->decoder);
     trace->metadata = NULL;
     delete metadata;
 }