From 5fd2e9fda6185e989583e6e61b9312683149747e Mon Sep 17 00:00:00 2001 From: Philippe Proulx Date: Thu, 11 Feb 2016 19:27:42 -0500 Subject: [PATCH] ir: move the stream event ctx field to the event MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Just like the event header, the stream event context field has its type defined in the stream class, but its data can be different in each event. The current strategy to sample and copy the current stream's stream event context and append the copies to an array is fine as far as CTF writer goes, but this array would not be populated strictly using the event in the context of Babeltrace components. Signed-off-by: Philippe Proulx Signed-off-by: Jérémie Galarneau --- formats/ctf/ir/event.c | 88 ++++++++++++++++-- formats/ctf/ir/stream.c | 99 ++------------------- include/babeltrace/ctf-ir/event-internal.h | 1 + include/babeltrace/ctf-ir/event.h | 22 +++++ include/babeltrace/ctf-ir/stream-internal.h | 2 - include/babeltrace/ctf-ir/stream.h | 25 ------ tests/lib/test_ctf_writer.c | 42 +++++---- 7 files changed, 131 insertions(+), 148 deletions(-) diff --git a/formats/ctf/ir/event.c b/formats/ctf/ir/event.c index 82ed0d94..6d8f57b3 100644 --- a/formats/ctf/ir/event.c +++ b/formats/ctf/ir/event.c @@ -61,6 +61,7 @@ struct bt_ctf_event *bt_ctf_event_create(struct bt_ctf_event_class *event_class) struct bt_ctf_field_type *event_context_type = NULL; struct bt_ctf_field_type *event_payload_type = NULL; struct bt_ctf_field *event_header = NULL; + struct bt_ctf_field *stream_event_context = NULL; struct bt_ctf_field *event_context = NULL; struct bt_ctf_field *event_payload = NULL; struct bt_value *environment = NULL; @@ -115,7 +116,6 @@ struct bt_ctf_event *bt_ctf_event_create(struct bt_ctf_event_class *event_class) BT_PUT(stream_event_ctx_type); BT_PUT(event_context_type); BT_PUT(event_payload_type); - if (ret) { /* * This means something went wrong during the validation @@ -153,11 +153,18 @@ struct bt_ctf_event *bt_ctf_event_create(struct bt_ctf_event_class *event_class) event->event_class = bt_get(event_class); event_header = bt_ctf_field_create(validation_output.event_header_type); - if (!event_header) { goto error; } + if (validation_output.stream_event_ctx_type) { + stream_event_context = bt_ctf_field_create( + validation_output.stream_event_ctx_type); + if (!stream_event_context) { + goto error; + } + } + if (validation_output.event_context_type) { event_context = bt_ctf_field_create( validation_output.event_context_type); @@ -183,6 +190,7 @@ struct bt_ctf_event *bt_ctf_event_create(struct bt_ctf_event_class *event_class) bt_ctf_validation_replace_types(trace, stream_class, event_class, &validation_output, validation_flags); BT_MOVE(event->event_header, event_header); + BT_MOVE(event->stream_event_context, stream_event_context); BT_MOVE(event->context_payload, event_context); BT_MOVE(event->fields_payload, event_payload); @@ -216,6 +224,7 @@ error: BT_PUT(stream_class); BT_PUT(trace); BT_PUT(event_header); + BT_PUT(stream_event_context); BT_PUT(event_context); BT_PUT(event_payload); assert(!packet_header_type); @@ -485,6 +494,54 @@ end: return ret; } +struct bt_ctf_field *bt_ctf_event_get_stream_event_context( + struct bt_ctf_event *event) +{ + struct bt_ctf_field *stream_event_context = NULL; + + if (!event || !event->stream_event_context) { + goto end; + } + + stream_event_context = event->stream_event_context; +end: + return bt_get(stream_event_context); +} + +int bt_ctf_event_set_stream_event_context(struct bt_ctf_event *event, + struct bt_ctf_field *stream_event_context) +{ + int ret = 0; + struct bt_ctf_field_type *field_type = NULL; + struct bt_ctf_stream_class *stream_class = NULL; + + if (!event || !stream_event_context) { + ret = -1; + goto end; + } + + stream_class = bt_ctf_event_class_get_stream_class(event->event_class); + /* + * We should not have been able to create the event without associating + * the event class to a stream class. + */ + assert(stream_class); + + field_type = bt_ctf_field_get_type(stream_event_context); + if (bt_ctf_field_type_compare(field_type, + stream_class->event_context_type)) { + ret = -1; + goto end; + } + + bt_get(stream_event_context); + BT_MOVE(event->stream_event_context, stream_event_context); +end: + BT_PUT(stream_class); + bt_put(field_type); + return ret; +} + void bt_ctf_event_get(struct bt_ctf_event *event) { bt_get(event); @@ -509,6 +566,7 @@ void bt_ctf_event_destroy(struct bt_object *obj) bt_put(event->event_class); } bt_put(event->event_header); + bt_put(event->stream_event_context); bt_put(event->context_payload); bt_put(event->fields_payload); g_free(event); @@ -563,6 +621,7 @@ int bt_ctf_event_validate(struct bt_ctf_event *event) { /* Make sure each field's payload has been set */ int ret; + struct bt_ctf_stream_class *stream_class = NULL; assert(event); ret = bt_ctf_field_validate(event->event_header); @@ -570,6 +629,19 @@ int bt_ctf_event_validate(struct bt_ctf_event *event) goto end; } + stream_class = bt_ctf_event_class_get_stream_class(event->event_class); + /* + * We should not have been able to create the event without associating + * the event class to a stream class. + */ + assert(stream_class); + if (stream_class->event_context_type) { + ret = bt_ctf_field_validate(event->stream_event_context); + if (ret) { + goto end; + } + } + ret = bt_ctf_field_validate(event->fields_payload); if (ret) { goto end; @@ -579,6 +651,7 @@ int bt_ctf_event_validate(struct bt_ctf_event *event) ret = bt_ctf_field_validate(event->context_payload); } end: + bt_put(stream_class); return ret; } @@ -679,16 +752,22 @@ struct bt_ctf_event *bt_ctf_event_copy(struct bt_ctf_event *event) if (event->event_header) { copy->event_header = bt_ctf_field_copy(event->event_header); - if (!copy->event_header) { goto error; } } + if (event->stream_event_context) { + copy->stream_event_context = + bt_ctf_field_copy(event->stream_event_context); + if (!copy->stream_event_context) { + goto error; + } + } + if (event->context_payload) { copy->context_payload = bt_ctf_field_copy( event->context_payload); - if (!copy->context_payload) { goto error; } @@ -696,7 +775,6 @@ struct bt_ctf_event *bt_ctf_event_copy(struct bt_ctf_event *event) if (event->fields_payload) { copy->fields_payload = bt_ctf_field_copy(event->fields_payload); - if (!copy->fields_payload) { goto error; } diff --git a/formats/ctf/ir/stream.c b/formats/ctf/ir/stream.c index 97f17946..517a11e6 100644 --- a/formats/ctf/ir/stream.c +++ b/formats/ctf/ir/stream.c @@ -281,20 +281,6 @@ struct bt_ctf_stream *bt_ctf_stream_create( goto error; } - /* - * A stream class may not have a stream event context defined - * in which case this stream will never have a stream_event_context - * member since, after a stream's creation, the parent stream class - * is "frozen" (immutable). - */ - if (stream_class->event_context_type) { - stream->event_context = bt_ctf_field_create( - stream_class->event_context_type); - if (!stream->event_context) { - goto error; - } - } - /* Initialize events_discarded */ ret = set_structure_field_integer(stream->packet_context, "events_discarded", 0); @@ -310,13 +296,6 @@ struct bt_ctf_stream *bt_ctf_stream_create( if (!stream->events) { goto error; } - if (stream_class->event_context_type) { - stream->event_contexts = g_ptr_array_new_with_free_func( - (GDestroyNotify) bt_put); - if (!stream->event_contexts) { - goto error; - } - } /* A trace is not allowed to have a NULL packet header */ assert(trace->packet_header_type); @@ -498,7 +477,6 @@ int bt_ctf_stream_append_event(struct bt_ctf_stream *stream, struct bt_ctf_event *event) { int ret = 0; - struct bt_ctf_field *event_context_copy = NULL; if (!stream || !event) { ret = -1; @@ -511,32 +489,15 @@ int bt_ctf_stream_append_event(struct bt_ctf_stream *stream, goto end; } - /* Make sure the event's payload is set */ + /* Make sure the various scopes of the event are set */ ret = bt_ctf_event_validate(event); if (ret) { goto end; } - /* Sample the current stream event context by copying it */ - if (stream->event_context) { - /* Make sure the event context's payload is set */ - ret = bt_ctf_field_validate(stream->event_context); - if (ret) { - goto end; - } - - event_context_copy = bt_ctf_field_copy(stream->event_context); - if (!event_context_copy) { - ret = -1; - goto end; - } - } - - /* Save the new event along with its associated stream event context */ + /* Save the new event */ g_ptr_array_add(stream->events, event); - if (event_context_copy) { - g_ptr_array_add(stream->event_contexts, event_context_copy); - } + /* * Event had to hold a reference to its event class as long as it wasn't * part of the same trace hierarchy. From now on, the event and its @@ -598,49 +559,6 @@ end: return ret; } -struct bt_ctf_field *bt_ctf_stream_get_event_context( - struct bt_ctf_stream *stream) -{ - struct bt_ctf_field *event_context = NULL; - - if (!stream) { - goto end; - } - - event_context = stream->event_context; - if (event_context) { - bt_get(event_context); - } -end: - return event_context; -} - -int bt_ctf_stream_set_event_context(struct bt_ctf_stream *stream, - struct bt_ctf_field *field) -{ - int ret = 0; - struct bt_ctf_field_type *field_type = NULL; - - if (!stream || !field) { - ret = -1; - goto end; - } - - field_type = bt_ctf_field_get_type(field); - if (bt_ctf_field_type_compare(field_type, - stream->stream_class->event_context_type)) { - ret = -1; - goto end; - } - - bt_get(field); - bt_put(stream->event_context); - stream->event_context = field; -end: - bt_put(field_type); - return ret; -} - struct bt_ctf_field *bt_ctf_stream_get_packet_header( struct bt_ctf_stream *stream) { @@ -843,10 +761,9 @@ int bt_ctf_stream_flush(struct bt_ctf_stream *stream) } /* Write stream event context */ - if (stream->event_contexts) { + if (event->stream_event_context) { ret = bt_ctf_field_serialize( - g_ptr_array_index(stream->event_contexts, i), - &stream->pos); + event->stream_event_context, &stream->pos); if (ret) { goto end; } @@ -885,9 +802,6 @@ int bt_ctf_stream_flush(struct bt_ctf_stream *stream) } g_ptr_array_set_size(stream->events, 0); - if (stream->event_contexts) { - g_ptr_array_set_size(stream->event_contexts, 0); - } stream->flushed_packet_count++; end: bt_put(integer); @@ -918,9 +832,6 @@ void bt_ctf_stream_destroy(struct bt_object *obj) if (stream->events) { g_ptr_array_free(stream->events, TRUE); } - if (stream->event_contexts) { - g_ptr_array_free(stream->event_contexts, TRUE); - } bt_put(stream->packet_header); bt_put(stream->packet_context); bt_put(stream->event_context); diff --git a/include/babeltrace/ctf-ir/event-internal.h b/include/babeltrace/ctf-ir/event-internal.h index c68316c1..f381be8d 100644 --- a/include/babeltrace/ctf-ir/event-internal.h +++ b/include/babeltrace/ctf-ir/event-internal.h @@ -41,6 +41,7 @@ struct bt_ctf_event { struct bt_object base; struct bt_ctf_event_class *event_class; struct bt_ctf_field *event_header; + struct bt_ctf_field *stream_event_context; struct bt_ctf_field *context_payload; struct bt_ctf_field *fields_payload; }; diff --git a/include/babeltrace/ctf-ir/event.h b/include/babeltrace/ctf-ir/event.h index 4b061d62..5b2e6f99 100644 --- a/include/babeltrace/ctf-ir/event.h +++ b/include/babeltrace/ctf-ir/event.h @@ -219,6 +219,28 @@ extern struct bt_ctf_field *bt_ctf_event_get_event_context( extern int bt_ctf_event_set_event_context(struct bt_ctf_event *event, struct bt_ctf_field *context); +/* + * bt_ctf_event_get_stream_event_context: Get an event's stream event context + * + * @param event_class Event class. + * + * Returns a field on success (a structure), NULL on error. + */ +extern struct bt_ctf_field *bt_ctf_event_get_stream_event_context( + struct bt_ctf_event *event); + +/* + * bt_ctf_event_set_stream_event_context: Set an event's stream event context + * + * @param event Event. + * @param context Event stream context field (must match the stream class' + * stream event context type). + * + * Returns 0 on success, a negative value on error. + */ +extern int bt_ctf_event_set_stream_event_context(struct bt_ctf_event *event, + struct bt_ctf_field *context); + /* * bt_ctf_event_copy: Deep-copy an event. * diff --git a/include/babeltrace/ctf-ir/stream-internal.h b/include/babeltrace/ctf-ir/stream-internal.h index 25c526c0..7461d321 100644 --- a/include/babeltrace/ctf-ir/stream-internal.h +++ b/include/babeltrace/ctf-ir/stream-internal.h @@ -42,8 +42,6 @@ struct bt_ctf_stream { struct bt_ctf_stream_class *stream_class; /* Array of pointers to bt_ctf_event for the current packet */ GPtrArray *events; - /* Array of pointers to bt_ctf_field associated with each event */ - GPtrArray *event_contexts; struct ctf_stream_pos pos; unsigned int flushed_packet_count; struct bt_ctf_field *packet_header; diff --git a/include/babeltrace/ctf-ir/stream.h b/include/babeltrace/ctf-ir/stream.h index 18f86a7c..ba43f0d5 100644 --- a/include/babeltrace/ctf-ir/stream.h +++ b/include/babeltrace/ctf-ir/stream.h @@ -173,31 +173,6 @@ extern int bt_ctf_stream_set_event_header( struct bt_ctf_stream *stream, struct bt_ctf_field *event_header); -/* - * bt_ctf_stream_get_event_context: get a stream's event context. - * - * @param stream Stream instance. - * - * Returns a field instance on success, NULL on error. - */ -extern struct bt_ctf_field *bt_ctf_stream_get_event_context( - struct bt_ctf_stream *stream); - -/* - * bt_ctf_stream_set_event_context: set a stream's event context. - * - * The event context's type must match the stream class' event - * context type. - * - * @param stream Stream instance. - * @param event_context Event context field instance. - * - * Returns a field instance on success, NULL on error. - */ -extern int bt_ctf_stream_set_event_context( - struct bt_ctf_stream *stream, - struct bt_ctf_field *event_context); - /* * bt_ctf_stream_flush: flush a stream. * diff --git a/tests/lib/test_ctf_writer.c b/tests/lib/test_ctf_writer.c index 8d8ffa91..1e7393a3 100644 --- a/tests/lib/test_ctf_writer.c +++ b/tests/lib/test_ctf_writer.c @@ -58,7 +58,7 @@ #define DEFAULT_CLOCK_TIME 0 #define DEFAULT_CLOCK_VALUE 0 -#define NR_TESTS 597 +#define NR_TESTS 591 static int64_t current_time = 42; @@ -695,7 +695,9 @@ void append_simple_event(struct bt_ctf_stream_class *stream_class, ok(bt_ctf_clock_set_time(clock, current_time) == 0, "Set clock time"); /* Populate stream event context */ - stream_event_context = bt_ctf_stream_get_event_context(stream); + stream_event_context = + bt_ctf_event_get_stream_event_context(simple_event); + assert(stream_event_context); stream_event_context_field = bt_ctf_field_structure_get_field( stream_event_context, "common_event_context"); bt_ctf_field_unsigned_integer_set_value(stream_event_context_field, 42); @@ -825,7 +827,8 @@ void append_complex_event(struct bt_ctf_stream_class *stream_class, struct bt_ctf_field *uint_35_field, *int_16_field, *a_string_field, *inner_structure_field, *complex_structure_field, *a_sequence_field, *enum_variant_field, *enum_container_field, - *variant_field, *an_array_field, *ret_field; + *variant_field, *an_array_field, *stream_event_ctx_field, + *stream_event_ctx_int_field, *ret_field; uint64_t ret_unsigned_int; int64_t ret_signed_int; const char *ret_string; @@ -1307,6 +1310,14 @@ void append_complex_event(struct bt_ctf_stream_class *stream_class, bt_put(int_16_field); } + stream_event_ctx_field = bt_ctf_event_get_stream_event_context(event); + assert(stream_event_ctx_field); + stream_event_ctx_int_field = bt_ctf_field_structure_get_field( + stream_event_ctx_field, "common_event_context"); + BT_PUT(stream_event_ctx_field); + bt_ctf_field_unsigned_integer_set_value(stream_event_ctx_int_field, 17); + BT_PUT(stream_event_ctx_int_field); + bt_ctf_clock_set_time(clock, ++current_time); ok(bt_ctf_stream_append_event(stream, event) == 0, "Append a complex event to a stream"); @@ -2181,7 +2192,7 @@ void packet_resize_test(struct bt_ctf_stream_class *stream_class, uint64_t ret_uint64; int events_appended = 0; struct bt_ctf_field *packet_context = NULL, - *packet_context_field = NULL, *event_context = NULL; + *packet_context_field = NULL, *stream_event_context = NULL; struct bt_ctf_field_type *ep_field_1_type = NULL; struct bt_ctf_field_type *ep_a_string_type = NULL; struct bt_ctf_field_type *ep_type = NULL; @@ -2224,22 +2235,6 @@ void packet_resize_test(struct bt_ctf_stream_class *stream_class, "bt_ctf_event_get_payload_by_index handles an invalid index correctly"); bt_put(event); - ok(bt_ctf_stream_get_event_context(NULL) == NULL, - "bt_ctf_stream_get_event_context handles NULL correctly"); - event_context = bt_ctf_stream_get_event_context(stream); - ok(event_context, - "bt_ctf_stream_get_event_context returns a stream event context"); - ok(bt_ctf_stream_set_event_context(NULL, event_context) < 0, - "bt_ctf_stream_set_event_context handles a NULL stream correctly"); - ok(bt_ctf_stream_set_event_context(stream, NULL) < 0, - "bt_ctf_stream_set_event_context handles a NULL stream event context correctly"); - ok(!bt_ctf_stream_set_event_context(stream, event_context), - "bt_ctf_stream_set_event_context correctly set a stream event context"); - ret_field = bt_ctf_field_create(ep_field_1_type); - ok(bt_ctf_stream_set_event_context(stream, ret_field) < 0, - "bt_ctf_stream_set_event_context rejects an event context of incorrect type"); - bt_put(ret_field); - for (i = 0; i < PACKET_RESIZE_TEST_LENGTH; i++) { event = bt_ctf_event_create(event_class); struct bt_ctf_field *integer = @@ -2258,8 +2253,11 @@ void packet_resize_test(struct bt_ctf_stream_class *stream_class, bt_put(string); /* Populate stream event context */ - integer = bt_ctf_field_structure_get_field(event_context, + stream_event_context = + bt_ctf_event_get_stream_event_context(event); + integer = bt_ctf_field_structure_get_field(stream_event_context, "common_event_context"); + BT_PUT(stream_event_context); ret |= bt_ctf_field_unsigned_integer_set_value(integer, i % 42); bt_put(integer); @@ -2306,7 +2304,7 @@ end: bt_put(string_type); bt_put(packet_context); bt_put(packet_context_field); - bt_put(event_context); + bt_put(stream_event_context); bt_put(event_class); bt_put(ep_field_1_type); bt_put(ep_a_string_type); -- 2.34.1