From 1fa280c9d54077c5b810780c335ba6226207e82a Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Mon, 11 Dec 2023 12:21:12 -0500 Subject: [PATCH] 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 Reviewed-on: https://review.lttng.org/c/babeltrace/+/8167 Reviewed-by: Philippe Proulx Reviewed-on: https://review.lttng.org/c/babeltrace/+/12270 Tested-by: jenkins --- .../ctf/common/src/metadata/tsdl/decoder.cpp | 9 +++++++-- .../ctf/common/src/metadata/tsdl/decoder.hpp | 9 ++++++++- src/plugins/ctf/fs-src/fs.hpp | 3 ++- src/plugins/ctf/fs-src/metadata.cpp | 10 ++++------ src/plugins/ctf/fs-src/query.cpp | 15 ++++++--------- src/plugins/ctf/lttng-live/data-stream.cpp | 4 ++-- src/plugins/ctf/lttng-live/lttng-live.hpp | 3 ++- src/plugins/ctf/lttng-live/metadata.cpp | 8 +++----- 8 files changed, 34 insertions(+), 27 deletions(-) diff --git a/src/plugins/ctf/common/src/metadata/tsdl/decoder.cpp b/src/plugins/ctf/common/src/metadata/tsdl/decoder.cpp index 739c648b..b6fe5174 100644 --- 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 4460acae..0aee6173 100644 --- 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; + /* * 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 54a8ec02..1cb9e8b0 100644 --- 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 cebd21fc..cbe4172b 100644 --- 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 95453ada..6625ec7d 100644 --- 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 c83d8491..5ba068c3 100644 --- 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 7d93326f..8016efc2 100644 --- 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 6aaa7beb..e90b352e 100644 --- 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; } -- 2.34.1