From 8480c8cc7e985169ab42060d3cd3c72d6c8d240d Mon Sep 17 00:00:00 2001 From: Philippe Proulx Date: Fri, 21 Jul 2017 16:13:16 -0400 Subject: [PATCH] ir: trace: pass remove listeners when adding listeners MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit With this patch, when you call bt_ctf_trace_add_is_static_listener(), you can also provide a "remove listener" which is called when the added listener is removed, weither manually with bt_ctf_trace_remove_is_static_listener(), or when the trace object is destroyed. This is useful when you dynamically allocate data only for the listener to add and you are not the only owner of the trace object: the remove listener is where to free/release this data. Existing plugins and tests are modified to match and test this change. Signed-off-by: Philippe Proulx Signed-off-by: Jérémie Galarneau --- include/babeltrace/ctf-ir/trace-internal.h | 1 + include/babeltrace/ctf-ir/trace.h | 42 +++++++++++--- lib/ctf-ir/trace.c | 66 +++++++++++++++++++--- plugins/ctf/fs-sink/write.c | 2 +- plugins/lttng-utils/copy.c | 2 +- tests/lib/test_ctf_writer.c | 34 ++++++----- 6 files changed, 116 insertions(+), 31 deletions(-) diff --git a/include/babeltrace/ctf-ir/trace-internal.h b/include/babeltrace/ctf-ir/trace-internal.h index 468b72f0..f2275fb7 100644 --- a/include/babeltrace/ctf-ir/trace-internal.h +++ b/include/babeltrace/ctf-ir/trace-internal.h @@ -71,6 +71,7 @@ struct bt_ctf_trace { GPtrArray *listeners; /* Array of struct listener_wrapper */ GArray *is_static_listeners; bt_bool is_static; + bt_bool in_remove_listener; }; struct metadata_context { diff --git a/include/babeltrace/ctf-ir/trace.h b/include/babeltrace/ctf-ir/trace.h index c7fa1f31..34ba4376 100644 --- a/include/babeltrace/ctf-ir/trace.h +++ b/include/babeltrace/ctf-ir/trace.h @@ -161,6 +161,20 @@ struct bt_ctf_clock_class; typedef void (* bt_ctf_trace_is_static_listener)( struct bt_ctf_trace *trace_class, void *data); +/** +@brief User function type to use with + bt_ctf_trace_add_is_static_listener(). + +@param[in] trace_class Trace class to which the listener was added. +@param[in] data User data as passed to + bt_ctf_trace_add_is_static_listener() when + you added the listener. + +@prenotnull{trace_class} +*/ +typedef void (* bt_ctf_trace_listener_removed)( + struct bt_ctf_trace *trace_class, void *data); + /** @name Creation function @{ @@ -873,6 +887,13 @@ extern int bt_ctf_trace_set_is_static(struct bt_ctf_trace *trace_class); \p listener is called with \p data, the user data, the first time bt_ctf_trace_set_is_static() is called on \p trace_class. +When the trace is destroyed, or when you remove the added listener with +bt_ctf_trace_remove_is_static_listener(), \p listener_removed is called +if it's not \c NULL. You can use \p listener_removed to free any dynamic +data which exists only for the added listener. You cannot call +any function which modifies \p trace_class during the execution of +\p listener_removed, including bt_ctf_trace_remove_is_static_listener(). + This function fails if \p trace_class is already static: you need to check the condition first with bt_ctf_trace_is_static(). @@ -881,12 +902,18 @@ listener within \p trace. You can use this identifier to remove the specific listener you added with bt_ctf_trace_remove_is_static_listener(). -@param[in] trace_class Trace class to which to add the listener. -@param[in] listener Listener to add to \p trace_class. -@param[in] data User data passed when \p listener is called. -@returns A unique numeric identifier for this listener - on success (0 or greater), or a negative value - on error. +@param[in] trace_class Trace class to which to add the + listener. +@param[in] listener Listener to add to \p trace_class. +@param[in] listener_removed Remove listener called when \p listener + is removed from \p trace_class, or + \c NULL if you don't need a remove + listener. +@param[in] data User data passed when \p listener or + \p listener_removed is called. +@returns A unique numeric identifier for this + listener on success (0 or greater), or a + negative value on error. @prenotnull{trace_class} @prenotnull{listener} @@ -902,7 +929,8 @@ bt_ctf_trace_remove_is_static_listener(). */ extern int bt_ctf_trace_add_is_static_listener( struct bt_ctf_trace *trace_class, - bt_ctf_trace_is_static_listener listener, void *data); + bt_ctf_trace_is_static_listener listener, + bt_ctf_trace_listener_removed listener_removed, void *data); /** @brief Removes the "trace is static" listener identified by diff --git a/lib/ctf-ir/trace.c b/lib/ctf-ir/trace.c index 110c20d4..9482e097 100644 --- a/lib/ctf-ir/trace.c +++ b/lib/ctf-ir/trace.c @@ -64,6 +64,7 @@ struct listener_wrapper { struct bt_ctf_trace_is_static_listener_elem { bt_ctf_trace_is_static_listener func; + bt_ctf_trace_listener_removed removed; void *data; }; @@ -273,6 +274,30 @@ void bt_ctf_trace_destroy(struct bt_object *obj) BT_LOGD("Destroying trace object: addr=%p, name=\"%s\"", trace, bt_ctf_trace_get_name(trace)); + /* + * Call remove listeners first so that everything else still + * exists in the trace. + */ + if (trace->is_static_listeners) { + size_t i; + + for (i = 0; i < trace->is_static_listeners->len; i++) { + struct bt_ctf_trace_is_static_listener_elem elem = + g_array_index(trace->is_static_listeners, + struct bt_ctf_trace_is_static_listener_elem, i); + + if (elem.removed) { + elem.removed(trace, elem.data); + } + } + + g_array_free(trace->is_static_listeners, TRUE); + } + + if (trace->listeners) { + g_ptr_array_free(trace->listeners, TRUE); + } + if (trace->environment) { BT_LOGD_STR("Destroying environment attributes."); bt_ctf_attributes_destroy(trace->environment); @@ -297,14 +322,6 @@ void bt_ctf_trace_destroy(struct bt_object *obj) g_ptr_array_free(trace->stream_classes, TRUE); } - if (trace->listeners) { - g_ptr_array_free(trace->listeners, TRUE); - } - - if (trace->is_static_listeners) { - g_array_free(trace->is_static_listeners, TRUE); - } - BT_LOGD_STR("Putting packet header field type."); bt_put(trace->packet_header_type); g_free(trace); @@ -2166,11 +2183,13 @@ end: } int bt_ctf_trace_add_is_static_listener(struct bt_ctf_trace *trace, - bt_ctf_trace_is_static_listener listener, void *data) + bt_ctf_trace_is_static_listener listener, + bt_ctf_trace_listener_removed listener_removed, void *data) { int i; struct bt_ctf_trace_is_static_listener_elem new_elem = { .func = listener, + .removed = listener_removed, .data = data, }; @@ -2194,6 +2213,14 @@ int bt_ctf_trace_add_is_static_listener(struct bt_ctf_trace *trace, goto end; } + if (trace->in_remove_listener) { + BT_LOGW("Cannot call this function during the execution of a remove listener: " + "addr=%p, name=\"%s\"", + trace, bt_ctf_trace_get_name(trace)); + i = -1; + goto end; + } + /* Find the next available spot */ for (i = 0; i < trace->is_static_listeners->len; i++) { struct bt_ctf_trace_is_static_listener_elem elem = @@ -2232,6 +2259,15 @@ int bt_ctf_trace_remove_is_static_listener( goto end; } + if (trace->in_remove_listener) { + BT_LOGW("Cannot call this function during the execution of a remove listener: " + "addr=%p, name=\"%s\", listener-id=%d", + trace, bt_ctf_trace_get_name(trace), + listener_id); + ret = -1; + goto end; + } + if (listener_id < 0) { BT_LOGW("Invalid listener ID: must be zero or positive: " "listener-id=%d", listener_id); @@ -2260,7 +2296,19 @@ int bt_ctf_trace_remove_is_static_listener( goto end; } + if (elem->removed) { + /* Call remove listener */ + BT_LOGV("Calling remove listener: " + "trace-addr=%p, trace-name=\"%s\", " + "listener-id=%d", trace, bt_ctf_trace_get_name(trace), + listener_id); + trace->in_remove_listener = BT_TRUE; + elem->removed(trace, elem->data); + trace->in_remove_listener = BT_FALSE; + } + elem->func = NULL; + elem->removed = NULL; elem->data = NULL; BT_LOGV("Removed \"trace is static\" listener: " "trace-addr=%p, trace-name=\"%s\", " diff --git a/plugins/ctf/fs-sink/write.c b/plugins/ctf/fs-sink/write.c index 995d344b..519eb2cb 100644 --- a/plugins/ctf/fs-sink/write.c +++ b/plugins/ctf/fs-sink/write.c @@ -360,7 +360,7 @@ struct fs_writer *insert_new_writer( fs_writer->static_listener_id = -1; } else { ret = bt_ctf_trace_add_is_static_listener(trace, - trace_is_static_listener, fs_writer); + trace_is_static_listener, NULL, fs_writer); assert(ret >= 0); fs_writer->static_listener_id = ret; } diff --git a/plugins/lttng-utils/copy.c b/plugins/lttng-utils/copy.c index f99d3831..3bbc0a17 100644 --- a/plugins/lttng-utils/copy.c +++ b/plugins/lttng-utils/copy.c @@ -753,7 +753,7 @@ struct debug_info_trace *insert_new_trace(struct debug_info_iterator *debug_it, bt_ctf_trace_set_is_static(writer_trace); } else { ret = bt_ctf_trace_add_is_static_listener(trace, - trace_is_static_listener, di_trace); + trace_is_static_listener, NULL, di_trace); assert(ret >= 0); di_trace->static_listener_id = ret; } diff --git a/tests/lib/test_ctf_writer.c b/tests/lib/test_ctf_writer.c index 5347afbf..31fcb626 100644 --- a/tests/lib/test_ctf_writer.c +++ b/tests/lib/test_ctf_writer.c @@ -60,7 +60,7 @@ #define DEFAULT_CLOCK_TIME 0 #define DEFAULT_CLOCK_VALUE 0 -#define NR_TESTS 622 +#define NR_TESTS 623 static int64_t current_time = 42; static unsigned int packet_resize_test_length = PACKET_RESIZE_TEST_DEF_LENGTH; @@ -2720,7 +2720,13 @@ void test_static_trace(void) static void trace_is_static_listener(struct bt_ctf_trace *trace, void *data) { - *((int *) data) = 1; + *((int *) data) |= 1; +} + +static +void trace_listener_removed(struct bt_ctf_trace *trace, void *data) +{ + *((int *) data) |= 2; } static @@ -2740,18 +2746,19 @@ void test_trace_is_static_listener(void) trace = bt_ctf_trace_create(); assert(trace); ret = bt_ctf_trace_add_is_static_listener(NULL, - trace_is_static_listener, &called1); + trace_is_static_listener, trace_listener_removed, &called1); ok(ret < 0, "bt_ctf_trace_add_is_static_listener() handles NULL (trace)"); - ret = bt_ctf_trace_add_is_static_listener(trace, NULL, &called1); + ret = bt_ctf_trace_add_is_static_listener(trace, NULL, + trace_listener_removed, &called1); ok(ret < 0, "bt_ctf_trace_add_is_static_listener() handles NULL (listener)"); listener1_id = bt_ctf_trace_add_is_static_listener(trace, - trace_is_static_listener, &called1); + trace_is_static_listener, trace_listener_removed, &called1); ok(listener1_id >= 0, "bt_ctf_trace_add_is_static_listener() succeeds (1)"); listener2_id = bt_ctf_trace_add_is_static_listener(trace, - trace_is_static_listener, &called2); + trace_is_static_listener, trace_listener_removed, &called2); ok(listener2_id >= 0, "bt_ctf_trace_add_is_static_listener() succeeds (2)"); listener3_id = bt_ctf_trace_add_is_static_listener(trace, - trace_is_static_listener, &called3); + trace_is_static_listener, trace_listener_removed, &called3); ok(listener3_id >= 0, "bt_ctf_trace_add_is_static_listener() succeeds (3)"); ret = bt_ctf_trace_remove_is_static_listener(NULL, 0); ok(ret < 0, "bt_ctf_trace_remove_is_static_listener() handles NULL (trace)"); @@ -2761,21 +2768,22 @@ void test_trace_is_static_listener(void) ok(ret < 0, "bt_ctf_trace_remove_is_static_listener() handles invalid ID (non existing)"); ret = bt_ctf_trace_remove_is_static_listener(trace, listener2_id); ok(ret == 0, "bt_ctf_trace_remove_is_static_listener() succeeds"); + ok(called2 == 2, "bt_ctf_trace_remove_is_static_listener() calls the remove listener"); listener4_id = bt_ctf_trace_add_is_static_listener(trace, - trace_is_static_listener, &called4); + trace_is_static_listener, NULL, &called4); ok(listener4_id >= 0, "bt_ctf_trace_add_is_static_listener() succeeds (4)"); ok(called1 == 0, "\"trace is static\" listener not called before the trace is made static (1)"); - ok(called2 == 0, "\"trace is static\" listener not called before the trace is made static (2)"); + ok(called2 == 2, "\"trace is static\" listener not called before the trace is made static (2)"); ok(called3 == 0, "\"trace is static\" listener not called before the trace is made static (3)"); ok(called4 == 0, "\"trace is static\" listener not called before the trace is made static (4)"); ret = bt_ctf_trace_set_is_static(trace); assert(ret == 0); ret = bt_ctf_trace_add_is_static_listener(trace, - trace_is_static_listener, &called1); + trace_is_static_listener, trace_listener_removed, &called1); ok(ret < 0, "bt_ctf_trace_add_is_static_listener() fails when the trace is static"); ok(called1 == 1, "\"trace is static\" listener called when the trace is made static (1)"); - ok(called2 == 0, "\"trace is static\" listener not called when the trace is made static (2)"); + ok(called2 == 2, "\"trace is static\" listener not called when the trace is made static (2)"); ok(called3 == 1, "\"trace is static\" listener called when the trace is made static (3)"); ok(called4 == 1, "\"trace is static\" listener called when the trace is made static (4)"); called1 = 0; @@ -2783,9 +2791,9 @@ void test_trace_is_static_listener(void) called3 = 0; called4 = 0; bt_put(trace); - ok(called1 == 0, "\"trace is static\" listener not called after the trace is put (1)"); + ok(called1 == 2, "\"trace is static\" listener not called after the trace is put (1)"); ok(called2 == 0, "\"trace is static\" listener not called after the trace is put (2)"); - ok(called3 == 0, "\"trace is static\" listener not called after the trace is put (3)"); + ok(called3 == 2, "\"trace is static\" listener not called after the trace is put (3)"); ok(called4 == 0, "\"trace is static\" listener not called after the trace is put (4)"); } -- 2.34.1