* [RFC PATCH] libtraceevent: Add tep_print_selected_fields()
@ 2021-08-02 11:27 Yordan Karadzhov (VMware)
2021-08-02 11:39 ` Yordan Karadzhov
2021-08-02 16:29 ` Steven Rostedt
0 siblings, 2 replies; 6+ messages in thread
From: Yordan Karadzhov (VMware) @ 2021-08-02 11:27 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel, Yordan Karadzhov (VMware)
The new method can print only a subset of the unique data fields of
the trace event. The print format is derived from the parsing tokens
(tep_print_parse objects) of the event. As a byproduct of this change
the existing method tep_print_fields() gets upgraded to use the
formats provided by the tokens.
Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
src/event-parse.c | 87 ++++++++++++++++++++++++++++++++++++++++++-----
src/event-parse.h | 3 ++
2 files changed, 81 insertions(+), 9 deletions(-)
diff --git a/src/event-parse.c b/src/event-parse.c
index f42ae38..7302f3d 100644
--- a/src/event-parse.c
+++ b/src/event-parse.c
@@ -3585,6 +3585,8 @@ tep_find_field(struct tep_event *event, const char *name)
return format;
}
+
+
/**
* tep_find_any_field - find any field by name
* @event: handle for the event
@@ -5333,6 +5335,19 @@ static int is_printable_array(char *p, unsigned int len)
return 1;
}
+static void dynamic_offset(struct tep_handle *tep,
+ struct tep_format_field *field,
+ void *data,
+ unsigned int *offset,
+ unsigned int *len)
+{
+ unsigned long long val;
+
+ val = tep_read_number(tep, data + field->offset, field->size);
+ *offset = val & SHRT_MAX;
+ *len = val >> 16;
+}
+
void tep_print_field(struct trace_seq *s, void *data,
struct tep_format_field *field)
{
@@ -5343,12 +5358,9 @@ void tep_print_field(struct trace_seq *s, void *data,
if (field->flags & TEP_FIELD_IS_ARRAY) {
offset = field->offset;
len = field->size;
- if (field->flags & TEP_FIELD_IS_DYNAMIC) {
- val = tep_read_number(tep, data + offset, len);
- offset = val;
- len = offset >> 16;
- offset &= 0xffff;
- }
+ if (field->flags & TEP_FIELD_IS_DYNAMIC)
+ dynamic_offset(tep, field, data, &offset, &len);
+
if (field->flags & TEP_FIELD_IS_STRING &&
is_printable_array(data + offset, len)) {
trace_seq_printf(s, "%s", (char *)data + offset);
@@ -5398,19 +5410,76 @@ void tep_print_field(struct trace_seq *s, void *data,
}
}
-void tep_print_fields(struct trace_seq *s, void *data,
- int size __maybe_unused, struct tep_event *event)
+static struct tep_print_parse *parse_format_next(struct tep_print_parse *parse)
+{
+ while (parse) {
+ if (strncmp(parse->format, "%", 1) == 0)
+ break;
+
+ parse = parse->next;
+ }
+
+ return parse;
+}
+
+static void tep_print_fmt_field(struct trace_seq *s, void *data,
+ const char *format,
+ struct tep_format_field *field)
+{
+ struct tep_handle *tep = field->event->tep;
+ unsigned int len, offset;
+ unsigned long long val;
+
+ if (field->flags & TEP_FIELD_IS_DYNAMIC) {
+ dynamic_offset(tep, field, data, &offset, &len);
+ if (len)
+ trace_seq_printf(s, format, (char *)data + offset);
+ else
+ trace_seq_printf(s, format, "(nil)");
+ } else {
+ val = tep_read_number(tep, data + field->offset, field->size);
+ trace_seq_printf(s, format, val);
+ }
+}
+
+void tep_print_selected_fields(struct trace_seq *s, void *data,
+ struct tep_event *event,
+ int ignore_mask)
{
struct tep_format_field *field;
+ struct tep_print_parse *parse;
+ unsigned int len;
+ int field_mask = 1;
+ parse = event->print_fmt.print_cache;
field = event->format.fields;
while (field) {
+ parse = parse_format_next(parse);
+
+ if (field_mask & ignore_mask)
+ goto next;
+
trace_seq_printf(s, " %s=", field->name);
- tep_print_field(s, data, field);
+
+ len = strlen(parse->format);
+ if (len > 0 && parse->format[len - 1] == 'x')
+ trace_seq_printf(s, "0x");
+
+ tep_print_fmt_field(s, data, parse->format, field);
+
+ next:
field = field->next;
+ parse = parse->next;
+ field_mask *= 2;
}
}
+void tep_print_fields(struct trace_seq *s, void *data,
+ int size __maybe_unused, struct tep_event *event)
+{
+ tep_print_selected_fields(s, data, event, 0);
+}
+
static int print_function(struct trace_seq *s, const char *format,
void *data, int size, struct tep_event *event,
struct tep_print_arg *arg)
diff --git a/src/event-parse.h b/src/event-parse.h
index d4a876f..fe0fbf4 100644
--- a/src/event-parse.h
+++ b/src/event-parse.h
@@ -547,6 +547,9 @@ void tep_print_field(struct trace_seq *s, void *data,
struct tep_format_field *field);
void tep_print_fields(struct trace_seq *s, void *data,
int size __maybe_unused, struct tep_event *event);
+void tep_print_selected_fields(struct trace_seq *s, void *data,
+ struct tep_event *event,
+ int ignore_mask);
int tep_strerror(struct tep_handle *tep, enum tep_errno errnum,
char *buf, size_t buflen);
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] libtraceevent: Add tep_print_selected_fields()
2021-08-02 11:27 [RFC PATCH] libtraceevent: Add tep_print_selected_fields() Yordan Karadzhov (VMware)
@ 2021-08-02 11:39 ` Yordan Karadzhov
2021-08-02 16:30 ` Steven Rostedt
2021-08-02 16:29 ` Steven Rostedt
1 sibling, 1 reply; 6+ messages in thread
From: Yordan Karadzhov @ 2021-08-02 11:39 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel
Steven,
The implementation of static void tep_print_fmt_field() is incomplete.
If you compare the logic there with the implementation of
void tep_print_field() you will see that I am not handling the case of
a field that is an "ARRAY".
Any ideas what is the best way to handle this case?
Thanks!
Yordan
On 2.08.21 г. 14:27, Yordan Karadzhov (VMware) wrote:
> The new method can print only a subset of the unique data fields of
> the trace event. The print format is derived from the parsing tokens
> (tep_print_parse objects) of the event. As a byproduct of this change
> the existing method tep_print_fields() gets upgraded to use the
> formats provided by the tokens.
>
> Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
> ---
> src/event-parse.c | 87 ++++++++++++++++++++++++++++++++++++++++++-----
> src/event-parse.h | 3 ++
> 2 files changed, 81 insertions(+), 9 deletions(-)
>
> diff --git a/src/event-parse.c b/src/event-parse.c
> index f42ae38..7302f3d 100644
> --- a/src/event-parse.c
> +++ b/src/event-parse.c
> @@ -3585,6 +3585,8 @@ tep_find_field(struct tep_event *event, const char *name)
> return format;
> }
>
> +
> +
> /**
> * tep_find_any_field - find any field by name
> * @event: handle for the event
> @@ -5333,6 +5335,19 @@ static int is_printable_array(char *p, unsigned int len)
> return 1;
> }
>
> +static void dynamic_offset(struct tep_handle *tep,
> + struct tep_format_field *field,
> + void *data,
> + unsigned int *offset,
> + unsigned int *len)
> +{
> + unsigned long long val;
> +
> + val = tep_read_number(tep, data + field->offset, field->size);
> + *offset = val & SHRT_MAX;
> + *len = val >> 16;
> +}
> +
> void tep_print_field(struct trace_seq *s, void *data,
> struct tep_format_field *field)
> {
> @@ -5343,12 +5358,9 @@ void tep_print_field(struct trace_seq *s, void *data,
> if (field->flags & TEP_FIELD_IS_ARRAY) {
> offset = field->offset;
> len = field->size;
> - if (field->flags & TEP_FIELD_IS_DYNAMIC) {
> - val = tep_read_number(tep, data + offset, len);
> - offset = val;
> - len = offset >> 16;
> - offset &= 0xffff;
> - }
> + if (field->flags & TEP_FIELD_IS_DYNAMIC)
> + dynamic_offset(tep, field, data, &offset, &len);
> +
> if (field->flags & TEP_FIELD_IS_STRING &&
> is_printable_array(data + offset, len)) {
> trace_seq_printf(s, "%s", (char *)data + offset);
> @@ -5398,19 +5410,76 @@ void tep_print_field(struct trace_seq *s, void *data,
> }
> }
>
> -void tep_print_fields(struct trace_seq *s, void *data,
> - int size __maybe_unused, struct tep_event *event)
> +static struct tep_print_parse *parse_format_next(struct tep_print_parse *parse)
> +{
> + while (parse) {
> + if (strncmp(parse->format, "%", 1) == 0)
> + break;
> +
> + parse = parse->next;
> + }
> +
> + return parse;
> +}
> +
> +static void tep_print_fmt_field(struct trace_seq *s, void *data,
> + const char *format,
> + struct tep_format_field *field)
> +{
> + struct tep_handle *tep = field->event->tep;
> + unsigned int len, offset;
> + unsigned long long val;
> +
> + if (field->flags & TEP_FIELD_IS_DYNAMIC) {
> + dynamic_offset(tep, field, data, &offset, &len);
> + if (len)
> + trace_seq_printf(s, format, (char *)data + offset);
> + else
> + trace_seq_printf(s, format, "(nil)");
> + } else {
> + val = tep_read_number(tep, data + field->offset, field->size);
> + trace_seq_printf(s, format, val);
> + }
> +}
> +
> +void tep_print_selected_fields(struct trace_seq *s, void *data,
> + struct tep_event *event,
> + int ignore_mask)
> {
> struct tep_format_field *field;
> + struct tep_print_parse *parse;
> + unsigned int len;
> + int field_mask = 1;
>
> + parse = event->print_fmt.print_cache;
> field = event->format.fields;
> while (field) {
> + parse = parse_format_next(parse);
> +
> + if (field_mask & ignore_mask)
> + goto next;
> +
> trace_seq_printf(s, " %s=", field->name);
> - tep_print_field(s, data, field);
> +
> + len = strlen(parse->format);
> + if (len > 0 && parse->format[len - 1] == 'x')
> + trace_seq_printf(s, "0x");
> +
> + tep_print_fmt_field(s, data, parse->format, field);
> +
> + next:
> field = field->next;
> + parse = parse->next;
> + field_mask *= 2;
> }
> }
>
> +void tep_print_fields(struct trace_seq *s, void *data,
> + int size __maybe_unused, struct tep_event *event)
> +{
> + tep_print_selected_fields(s, data, event, 0);
> +}
> +
> static int print_function(struct trace_seq *s, const char *format,
> void *data, int size, struct tep_event *event,
> struct tep_print_arg *arg)
> diff --git a/src/event-parse.h b/src/event-parse.h
> index d4a876f..fe0fbf4 100644
> --- a/src/event-parse.h
> +++ b/src/event-parse.h
> @@ -547,6 +547,9 @@ void tep_print_field(struct trace_seq *s, void *data,
> struct tep_format_field *field);
> void tep_print_fields(struct trace_seq *s, void *data,
> int size __maybe_unused, struct tep_event *event);
> +void tep_print_selected_fields(struct trace_seq *s, void *data,
> + struct tep_event *event,
> + int ignore_mask);
> int tep_strerror(struct tep_handle *tep, enum tep_errno errnum,
> char *buf, size_t buflen);
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] libtraceevent: Add tep_print_selected_fields()
2021-08-02 11:27 [RFC PATCH] libtraceevent: Add tep_print_selected_fields() Yordan Karadzhov (VMware)
2021-08-02 11:39 ` Yordan Karadzhov
@ 2021-08-02 16:29 ` Steven Rostedt
2021-08-03 12:42 ` Yordan Karadzhov
1 sibling, 1 reply; 6+ messages in thread
From: Steven Rostedt @ 2021-08-02 16:29 UTC (permalink / raw)
To: Yordan Karadzhov (VMware); +Cc: linux-trace-devel
On Mon, 2 Aug 2021 14:27:14 +0300
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
> The new method can print only a subset of the unique data fields of
> the trace event. The print format is derived from the parsing tokens
> (tep_print_parse objects) of the event. As a byproduct of this change
> the existing method tep_print_fields() gets upgraded to use the
> formats provided by the tokens.
Why add a new API? Just fix tep_print_field().
>
> Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
> ---
> src/event-parse.c | 87 ++++++++++++++++++++++++++++++++++++++++++-----
> src/event-parse.h | 3 ++
> 2 files changed, 81 insertions(+), 9 deletions(-)
>
> diff --git a/src/event-parse.c b/src/event-parse.c
> index f42ae38..7302f3d 100644
> --- a/src/event-parse.c
> +++ b/src/event-parse.c
> @@ -3585,6 +3585,8 @@ tep_find_field(struct tep_event *event, const char *name)
> return format;
> }
>
> +
> +
Spurious newlines?
> /**
> * tep_find_any_field - find any field by name
> * @event: handle for the event
> @@ -5333,6 +5335,19 @@ static int is_printable_array(char *p, unsigned int len)
> return 1;
> }
>
> +static void dynamic_offset(struct tep_handle *tep,
> + struct tep_format_field *field,
> + void *data,
> + unsigned int *offset,
> + unsigned int *len)
> +{
> + unsigned long long val;
> +
> + val = tep_read_number(tep, data + field->offset, field->size);
> + *offset = val & SHRT_MAX;
Don't use SHRT_MAX. Although it is the same, there's no connection
of it being a short number (it only matches by coincidence), and I
don't want to add any assumptions that this is a short. It's 16 bits,
and that's all it is. The fact that a short is also 16 bits is just a
coincidence ;-)
> + *len = val >> 16;
If anything, we should define:
#define TEP_OFFSET_MASK 0xffff
#define TEP_LEN_SHIFT 16
and then we can:
*offset = val & TEP_OFFSET_MASK;
*len = val >> TEP_LEN_SHIFT;
Which would make it much easier to read and document what it actually
is.
> +}
> +
Also, the above should be in a separate cleanup patch that that should
remove all the open coded conversions.
> void tep_print_field(struct trace_seq *s, void *data,
> struct tep_format_field *field)
> {
> @@ -5343,12 +5358,9 @@ void tep_print_field(struct trace_seq *s, void *data,
> if (field->flags & TEP_FIELD_IS_ARRAY) {
> offset = field->offset;
> len = field->size;
> - if (field->flags & TEP_FIELD_IS_DYNAMIC) {
> - val = tep_read_number(tep, data + offset, len);
> - offset = val;
> - len = offset >> 16;
> - offset &= 0xffff;
> - }
> + if (field->flags & TEP_FIELD_IS_DYNAMIC)
> + dynamic_offset(tep, field, data, &offset, &len);
> +
> if (field->flags & TEP_FIELD_IS_STRING &&
> is_printable_array(data + offset, len)) {
> trace_seq_printf(s, "%s", (char *)data + offset);
> @@ -5398,19 +5410,76 @@ void tep_print_field(struct trace_seq *s, void *data,
> }
> }
>
What I was talking about is to change tep_print_field() to do something like:
Take the current tep_print_field() and turn it into static _tep_print_field().
[ Not even compiled tested ]
void tep_print_field(struct trace_seq *s, void *data,
struct tep_format_field *field)
{
struct tep_event = field->event;
struct tep_print_parse = event->print_fmt.print_cache;
struct tep_handle *tep = event->tep;
unsigned int offset, len;
if (event->flags & TEP_EVENT_FL_FAILED)
goto out;
if (field->flags & TEP_FIELD_IS_DYNAMIC)
dynamic_offset(tep, field, data, &offset, &len);
for (;parse; parse = parse->next) {
if (parse->type == PRINT_FMT_STRING)
continue;
if (parse->arg->type != TEP_PRINT_FIELD)
continue;
if (parse->arg->field.field->field != field)
continue;
print_parse_data(parse, s, data, size, event);
return;
}
out:
/* Not found */
_tep_print_field(s, data, field);
}
That print_parse_data() would be extracted from print_event_cache()
static int print_parse_data(struct tep_print_parse *parse, struct trace_seq *s,
void *data, int size, struct tep_event *event)
{
if (parse->len_as_arg)
len_arg = eval_num_arg(data, size, event, parse->len_as_arg);
switch (parse->type) {
case PRINT_FMT_ARG_DIGIT:
print_arg_number(s, parse->format,
parse->len_as_arg ? len_arg : -1, data,
size, parse->ls, event, parse->arg);
break;
case PRINT_FMT_ARG_POINTER:
print_arg_pointer(s, parse->format,
parse->len_as_arg ? len_arg : 1,
data, size, event, parse->arg);
break;
case PRINT_FMT_ARG_STRING:
print_arg_string(s, parse->format,
parse->len_as_arg ? len_arg : -1,
data, size, event, parse->arg);
break;
case PRINT_FMT_STRING:
default:
trace_seq_printf(s, "%s", parse->format);
/* Return 1 on non field */
return 1;
}
/* Return 0 on field being processed */
return 0;
}
static void print_event_cache(struct tep_print_parse *parse, struct trace_seq *s,
void *data, int size, struct tep_event *event)
{
int len_arg;
while (parse) {
parse = parse->next;
parse_parse_data(parse, s, data, size, event);
}
}
> -void tep_print_fields(struct trace_seq *s, void *data,
> - int size __maybe_unused, struct tep_event *event)
> +static struct tep_print_parse *parse_format_next(struct tep_print_parse *parse)
> +{
> + while (parse) {
> + if (strncmp(parse->format, "%", 1) == 0)
> + break;
> +
> + parse = parse->next;
> + }
> +
> + return parse;
> +}
> +
> +static void tep_print_fmt_field(struct trace_seq *s, void *data,
> + const char *format,
> + struct tep_format_field *field)
> +{
> + struct tep_handle *tep = field->event->tep;
> + unsigned int len, offset;
> + unsigned long long val;
> +
> + if (field->flags & TEP_FIELD_IS_DYNAMIC) {
> + dynamic_offset(tep, field, data, &offset, &len);
> + if (len)
> + trace_seq_printf(s, format, (char *)data + offset);
> + else
> + trace_seq_printf(s, format, "(nil)");
> + } else {
> + val = tep_read_number(tep, data + field->offset, field->size);
> + trace_seq_printf(s, format, val);
> + }
> +}
> +
> +void tep_print_selected_fields(struct trace_seq *s, void *data,
> + struct tep_event *event,
> + int ignore_mask)
> {
> struct tep_format_field *field;
> + struct tep_print_parse *parse;
> + unsigned int len;
> + int field_mask = 1;
>
> + parse = event->print_fmt.print_cache;
> field = event->format.fields;
> while (field) {
> + parse = parse_format_next(parse);
> +
> + if (field_mask & ignore_mask)
> + goto next;
> +
> trace_seq_printf(s, " %s=", field->name);
> - tep_print_field(s, data, field);
> +
> + len = strlen(parse->format);
> + if (len > 0 && parse->format[len - 1] == 'x')
> + trace_seq_printf(s, "0x");
> +
> + tep_print_fmt_field(s, data, parse->format, field);
> +
> + next:
> field = field->next;
> + parse = parse->next;
> + field_mask *= 2;
> }
> }
>
> +void tep_print_fields(struct trace_seq *s, void *data,
> + int size __maybe_unused, struct tep_event *event)
> +{
> + tep_print_selected_fields(s, data, event, 0);
> +}
> +
> static int print_function(struct trace_seq *s, const char *format,
> void *data, int size, struct tep_event *event,
> struct tep_print_arg *arg)
> diff --git a/src/event-parse.h b/src/event-parse.h
> index d4a876f..fe0fbf4 100644
> --- a/src/event-parse.h
> +++ b/src/event-parse.h
> @@ -547,6 +547,9 @@ void tep_print_field(struct trace_seq *s, void *data,
> struct tep_format_field *field);
> void tep_print_fields(struct trace_seq *s, void *data,
> int size __maybe_unused, struct tep_event *event);
> +void tep_print_selected_fields(struct trace_seq *s, void *data,
> + struct tep_event *event,
> + int ignore_mask);
Again, I'm not so big on the selected fields version. Just call
tep_print_field for each section, as it just adds to the trace_seq()
which was created for this purpose (to avoid having to enter a list).
-- Steve
> int tep_strerror(struct tep_handle *tep, enum tep_errno errnum,
> char *buf, size_t buflen);
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] libtraceevent: Add tep_print_selected_fields()
2021-08-02 11:39 ` Yordan Karadzhov
@ 2021-08-02 16:30 ` Steven Rostedt
0 siblings, 0 replies; 6+ messages in thread
From: Steven Rostedt @ 2021-08-02 16:30 UTC (permalink / raw)
To: Yordan Karadzhov; +Cc: linux-trace-devel
On Mon, 2 Aug 2021 14:39:11 +0300
Yordan Karadzhov <y.karadz@gmail.com> wrote:
> The implementation of static void tep_print_fmt_field() is incomplete.
> If you compare the logic there with the implementation of
> void tep_print_field() you will see that I am not handling the case of
> a field that is an "ARRAY".
>
> Any ideas what is the best way to handle this case?
I think my reply to your patch should cover this.
-- Steve
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] libtraceevent: Add tep_print_selected_fields()
2021-08-02 16:29 ` Steven Rostedt
@ 2021-08-03 12:42 ` Yordan Karadzhov
2021-08-03 13:48 ` Steven Rostedt
0 siblings, 1 reply; 6+ messages in thread
From: Yordan Karadzhov @ 2021-08-03 12:42 UTC (permalink / raw)
To: Steven Rostedt; +Cc: linux-trace-devel
On 2.08.21 г. 19:29, Steven Rostedt wrote:
> What I was talking about is to change tep_print_field() to do something like:
>
> Take the current tep_print_field() and turn it into static _tep_print_field().
>
> [ Not even compiled tested ]
>
Hi Steven,
I am not able to make sense from the code below.
> void tep_print_field(struct trace_seq *s, void *data,
> struct tep_format_field *field)
> {
> struct tep_event = field->event;
> struct tep_print_parse = event->print_fmt.print_cache;
> struct tep_handle *tep = event->tep;
> unsigned int offset, len;
>
> if (event->flags & TEP_EVENT_FL_FAILED)
> goto out;
>
> if (field->flags & TEP_FIELD_IS_DYNAMIC)
> dynamic_offset(tep, field, data, &offset, &len);
>
> for (;parse; parse = parse->next) {
You need
if (!parse->arg)
continue;
> if (parse->type == PRINT_FMT_STRING)
> continue;
> if (parse->arg->type != TEP_PRINT_FIELD)
> continue;
I can't understand the idea of those two checks. I printed the values and they don't seem to have any selective power.
> if (parse->arg->field.field->field != field)
> continue;
>
This does not compile. I guess you mean
if (parse->arg->field.field != field)
continue;
however "parse->arg->field.field" looks like unused memory (NULL or 0xffffffff) and this check always fails.
Maybe we must call some of the process_XXX()static functions first in order to make your new version of
tep_print_field() works?
Thanks!
Yordan
> print_parse_data(parse, s, data, size, event);
> return;
> }
>
> out:
> /* Not found */
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] libtraceevent: Add tep_print_selected_fields()
2021-08-03 12:42 ` Yordan Karadzhov
@ 2021-08-03 13:48 ` Steven Rostedt
0 siblings, 0 replies; 6+ messages in thread
From: Steven Rostedt @ 2021-08-03 13:48 UTC (permalink / raw)
To: Yordan Karadzhov; +Cc: linux-trace-devel
On Tue, 3 Aug 2021 15:42:02 +0300
Yordan Karadzhov <y.karadz@gmail.com> wrote:
> On 2.08.21 г. 19:29, Steven Rostedt wrote:
> > What I was talking about is to change tep_print_field() to do something like:
> >
> > Take the current tep_print_field() and turn it into static _tep_print_field().
> >
> > [ Not even compiled tested ]
> >
>
> Hi Steven,
>
> I am not able to make sense from the code below.
>
> > void tep_print_field(struct trace_seq *s, void *data,
> > struct tep_format_field *field)
> > {
> > struct tep_event = field->event;
> > struct tep_print_parse = event->print_fmt.print_cache;
> > struct tep_handle *tep = event->tep;
> > unsigned int offset, len;
> >
> > if (event->flags & TEP_EVENT_FL_FAILED)
> > goto out;
> >
> > if (field->flags & TEP_FIELD_IS_DYNAMIC)
> > dynamic_offset(tep, field, data, &offset, &len);
> >
> > for (;parse; parse = parse->next) {
>
> You need
> if (!parse->arg)
> continue;
Sure. Like I said, I never even compiled this ;-)
>
> > if (parse->type == PRINT_FMT_STRING)
> > continue;
> > if (parse->arg->type != TEP_PRINT_FIELD)
> > continue;
>
> I can't understand the idea of those two checks. I printed the values and they don't seem to have any selective power.
Well, the print_cache stores portions of the print fmt format string. Where if you have something like:
print fmt: "fd: 0x%08lx, offset: 0x%08lx, count: 0x%08lx", ((unsigned long)(REC->fd)), ((unsigned long)(REC->offset)), ((unsigned long)(REC->count))
The "fd: " part is a PRINT_FMT_STRING. We don't want anything to do with that.
But the cache part that has fields, should be an arg of type
TEP_PRINT_FIELD, which should be mapped to some field to print. That
is, one of the REC->fd, REC->offset or REC->count.
That's what you are looking to see how to print it. As it also contains
how that field should be printed normally.
>
> > if (parse->arg->field.field->field != field)
> > continue;
> >
>
> This does not compile. I guess you mean
> if (parse->arg->field.field != field)
> continue;
>
> however "parse->arg->field.field" looks like unused memory (NULL or 0xffffffff) and this check always fails.
>
> Maybe we must call some of the process_XXX()static functions first in order to make your new version of
> tep_print_field() works?
How are you calling it? After loading the tep with tracefs_local_events()?
Because it should be loaded via tep_parse_event(), which calll
parse_args() and load the args to the event.
Basically we should be doing what print_event_cache() does, but only
for the field we are interested in.
-- Steve
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-08-03 13:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-02 11:27 [RFC PATCH] libtraceevent: Add tep_print_selected_fields() Yordan Karadzhov (VMware)
2021-08-02 11:39 ` Yordan Karadzhov
2021-08-02 16:30 ` Steven Rostedt
2021-08-02 16:29 ` Steven Rostedt
2021-08-03 12:42 ` Yordan Karadzhov
2021-08-03 13:48 ` Steven Rostedt
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.