decoding tracef msg string via babeltrace API
diff mbox series

Message ID 5382125.x1mbapOiNS@agathebauer
State New
Headers show
Series
  • decoding tracef msg string via babeltrace API
Related show

Commit Message

Milian Wolff April 2, 2019, 7:40 p.m. UTC
On Dienstag, 2. April 2019 21:03:41 CEST Milian Wolff wrote:
> Hey all
> 
> I added a tracef tracepoint and want to decode it's `msg` field in the
> BT_EVENT_FIELDS scope. Babeltrace on the command line seems to handle this
> nicely, but whatever I try, it doesn't work with the API.
> 
> I'm using babeltrace from the stable-1.5 branch.
> 
> First, the output from babeltrace on the command line:
> 
> ```
> event {
>         name = "lttng_ust_tracef:event";
>         id = 9;
>         stream_id = 0;
>         loglevel = 14;
>         fields := struct {
>                 integer { size = 32; align = 8; signed = 0; encoding = none;
> base = 10; } __msg_length;
>                 integer { size = 8; align = 8; signed = 1; encoding = UTF8;
> base = 10; } _msg[ __msg_length ];
>         };
> };
> ...
> timestamp = 15:56:15.404973865, delta = +0.000023594, trace = 1000/64-bit,
> trace:hostname = agathebauer, trace:domain = ust, loglevel = TRACE_DEBUG
> (14), name = lttng_ust_tracef:event, stream.packet.context = {
> timestamp_begin = 4382372875047, timestamp_end = 4462741641629,
> content_size = 47177888, packet_size = 47185920, packet_seq_num = 0,
> events_discarded = 0, cpu_id = 3 }, stream.event.header = { id = (
> "compact" : container = 9 ), v = { compact = { timestamp = 70385656 } } },
> event.fields = { _msg_length = 33, msg = "run_event_entry: ImageCache,
> 1000" }
> ...
> ```
> 
> Now, in my code I find the event and then I've tried:
> 
> ```
>             auto definition = bt_ctf_get_field(event, scope, "msg");
>             auto declaration = bt_ctf_get_decl_from_def(definition);
>             auto type = bt_ctf_field_type(declaration);
>             auto encoding = bt_ctf_get_encoding(declaration);
>             auto string = bt_ctf_get_string(definition);
>             auto char_array = bt_ctf_get_char_array(definition);
>             const bt_definition* const* list = nullptr;
>             unsigned num_list = 0;
>             auto field_list_ret = bt_ctf_get_field_list(ctf_event,
> definition, &list, &num_list);
>             fprintf(stderr, "tracef dbg: %p %p | %d %d | %s | %s | %p %u
> %d\n", definition, declaration, type, encoding, string, char_array, list,
> num_list, field_list_ret);
> ```
> 
> the output is:
> 
> ```
> tracef dbg: 0x55e4c5b084f0 0x55e4c5b3f220 | 9 1 | (null) | (null) | (nil) 0
> -1
> 
> ```
> 
> So, it's a sequence (type 9), but I can't get the sequence... Actually, when
> I tried this the firs ttime without patching babeltrace, then it crashed in
> events.c:256 [1]. The def_sequence is non-null, but def_sequence->elems is
> null and that's not checked...
> 
> [1]: https://github.com/efficios/babeltrace/blob/stable-1.5/formats/ctf/
> events.c#L256
> 
> I'm quite stumped - what am I doing wrong? How does babeltrace handle the
> string-decoding of tracef's msg arg?

I've found out that babeltrace seems to not consume the public API. Instead, 
in `sequence.c` it's directly accessing the sequence's string member. If I 
patch `bt_ctf_get_string` like this:

```
+
+end:
 	return ret;
 }
```

it seems to be doing what I was hoping for. Would something like that be 
accepted upstream, or do we need a third string-returning function 
`bt_ctf_get_string_sequence` or similar?

Generally, I already dislike it very much that there is a distinction between 
`bt_ctf_get_string` and `bt_ctf_get_char_array` - both should - imo - be 
handled through `bt_ctf_get_string`.

Cheers

Comments

Milian Wolff April 2, 2019, 7:54 p.m. UTC | #1
On Dienstag, 2. April 2019 21:40:02 CEST Milian Wolff wrote:
> On Dienstag, 2. April 2019 21:03:41 CEST Milian Wolff wrote:
> > Hey all
> > 
> > I added a tracef tracepoint and want to decode it's `msg` field in the
> > BT_EVENT_FIELDS scope. Babeltrace on the command line seems to handle this
> > nicely, but whatever I try, it doesn't work with the API.
> > 
> > I'm using babeltrace from the stable-1.5 branch.
> > 
> > First, the output from babeltrace on the command line:
> > 
> > ```
> > event {
> > 
> >         name = "lttng_ust_tracef:event";
> >         id = 9;
> >         stream_id = 0;
> >         loglevel = 14;
> >         fields := struct {
> >         
> >                 integer { size = 32; align = 8; signed = 0; encoding =
> >                 none;
> > 
> > base = 10; } __msg_length;
> > 
> >                 integer { size = 8; align = 8; signed = 1; encoding =
> >                 UTF8;
> > 
> > base = 10; } _msg[ __msg_length ];
> > 
> >         };
> > 
> > };
> > ...
> > timestamp = 15:56:15.404973865, delta = +0.000023594, trace = 1000/64-bit,
> > trace:hostname = agathebauer, trace:domain = ust, loglevel = TRACE_DEBUG
> > (14), name = lttng_ust_tracef:event, stream.packet.context = {
> > timestamp_begin = 4382372875047, timestamp_end = 4462741641629,
> > content_size = 47177888, packet_size = 47185920, packet_seq_num = 0,
> > events_discarded = 0, cpu_id = 3 }, stream.event.header = { id = (
> > "compact" : container = 9 ), v = { compact = { timestamp = 70385656 } } },
> > event.fields = { _msg_length = 33, msg = "run_event_entry: ImageCache,
> > 1000" }
> > ...
> > ```
> > 
> > Now, in my code I find the event and then I've tried:
> > 
> > ```
> > 
> >             auto definition = bt_ctf_get_field(event, scope, "msg");
> >             auto declaration = bt_ctf_get_decl_from_def(definition);
> >             auto type = bt_ctf_field_type(declaration);
> >             auto encoding = bt_ctf_get_encoding(declaration);
> >             auto string = bt_ctf_get_string(definition);
> >             auto char_array = bt_ctf_get_char_array(definition);
> >             const bt_definition* const* list = nullptr;
> >             unsigned num_list = 0;
> >             auto field_list_ret = bt_ctf_get_field_list(ctf_event,
> > 
> > definition, &list, &num_list);
> > 
> >             fprintf(stderr, "tracef dbg: %p %p | %d %d | %s | %s | %p %u
> > 
> > %d\n", definition, declaration, type, encoding, string, char_array, list,
> > num_list, field_list_ret);
> > ```
> > 
> > the output is:
> > 
> > ```
> > tracef dbg: 0x55e4c5b084f0 0x55e4c5b3f220 | 9 1 | (null) | (null) | (nil)
> > 0
> > -1
> > 
> > ```
> > 
> > So, it's a sequence (type 9), but I can't get the sequence... Actually,
> > when I tried this the firs ttime without patching babeltrace, then it
> > crashed in events.c:256 [1]. The def_sequence is non-null, but
> > def_sequence->elems is null and that's not checked...
> > 
> > [1]: https://github.com/efficios/babeltrace/blob/stable-1.5/formats/ctf/
> > events.c#L256
> > 
> > I'm quite stumped - what am I doing wrong? How does babeltrace handle the
> > string-decoding of tracef's msg arg?
> 
> I've found out that babeltrace seems to not consume the public API. Instead,
> in `sequence.c` it's directly accessing the sequence's string member. If I
> patch `bt_ctf_get_string` like this:
> 
> ```
> diff --git a/formats/ctf/events.c b/formats/ctf/events.c
> index bd195b93..f0ea49c8 100644
> --- a/formats/ctf/events.c
> +++ b/formats/ctf/events.c
> @@ -599,12 +599,26 @@ end:
>  char *bt_ctf_get_string(const struct bt_definition *field)
>  {
>  	char *ret = NULL;
> +	const struct bt_declaration *decl = field ?
> bt_ctf_get_decl_from_def(field) : NULL;
> +	enum ctf_type_id type = decl ? bt_ctf_field_type(decl) :
> CTF_TYPE_UNKNOWN;
> 
> -	if (field && bt_ctf_field_type(bt_ctf_get_decl_from_def(field)) ==
> CTF_TYPE_STRING)
> +	if (type == CTF_TYPE_SEQUENCE) {
> +		enum ctf_string_encoding encoding = bt_ctf_get_encoding(decl);
> +		struct definition_sequence *sequence_definition;
> +		if (encoding == CTF_STRING_UTF8 || encoding  ==
> CTF_STRING_ASCII) {
> +			sequence_definition = container_of(field, struct
> definition_sequence, p);
> +			ret = sequence_definition->string->str;
> +			goto end;
> +		}
> +	} else if (type == CTF_TYPE_STRING) {
>  		ret = bt_get_string(field);
> -	else
> -		bt_ctf_field_set_error(-EINVAL);
> +		goto end;
> +	}
> 
> +error:
> +	bt_ctf_field_set_error(-EINVAL);
> +
> +end:
>  	return ret;
>  }
> ```
> 
> it seems to be doing what I was hoping for. Would something like that be
> accepted upstream, or do we need a third string-returning function
> `bt_ctf_get_string_sequence` or similar?
> 
> Generally, I already dislike it very much that there is a distinction
> between `bt_ctf_get_string` and `bt_ctf_get_char_array` - both should - imo
> - be handled through `bt_ctf_get_string`.

And now I found out that this is known and a patch is even available:

https://github.com/efficios/babeltrace/pull/98/commits/
3ab89a428ce2a8a4f76218f79c9eb6c26ae5aeee

It's been nearly 1.5 years and babeltrace 2.0 is still not ready. Can you 
please merge this into babeltrace 1.5 and release a new version of that stable 
branch?

Thanks

Patch
diff mbox series

diff --git a/formats/ctf/events.c b/formats/ctf/events.c
index bd195b93..f0ea49c8 100644
--- a/formats/ctf/events.c
+++ b/formats/ctf/events.c
@@ -599,12 +599,26 @@  end:
 char *bt_ctf_get_string(const struct bt_definition *field)
 {
 	char *ret = NULL;
+	const struct bt_declaration *decl = field ? 
bt_ctf_get_decl_from_def(field) : NULL;
+	enum ctf_type_id type = decl ? bt_ctf_field_type(decl) : 
CTF_TYPE_UNKNOWN;
 
-	if (field && bt_ctf_field_type(bt_ctf_get_decl_from_def(field)) == 
CTF_TYPE_STRING)
+	if (type == CTF_TYPE_SEQUENCE) {
+		enum ctf_string_encoding encoding = bt_ctf_get_encoding(decl);
+		struct definition_sequence *sequence_definition;
+		if (encoding == CTF_STRING_UTF8 || encoding  == 
CTF_STRING_ASCII) {
+			sequence_definition = container_of(field, struct 
definition_sequence, p);
+			ret = sequence_definition->string->str;
+			goto end;
+		}
+	} else if (type == CTF_TYPE_STRING) {
 		ret = bt_get_string(field);
-	else
-		bt_ctf_field_set_error(-EINVAL);
+		goto end;
+	}
 
+error:
+	bt_ctf_field_set_error(-EINVAL);