All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] Few patches, related to libtracevent APIs
@ 2019-03-22 13:07 Tzvetomir Stoyanov
  2019-03-22 13:07 ` [PATCH v2 1/3] tools/lib/traceevent: Change description of few APIs Tzvetomir Stoyanov
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Tzvetomir Stoyanov @ 2019-03-22 13:07 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

This patchset introduces new libtracevent APIs, which can be used for
accessing struct tep_handler fields. It also changes the description
of few APIs, to match more closely their logic.
There are also fixes to coding style problems, introduced by previous patches.

Tzvetomir Stoyanov (3):
  tools/lib/traceevent: Change description of few APIs
  tools/lib/traceevent: Coding style fixes
  tools/lib/traceevent: Implement new traceevent APIs for accessing
    struct tep_handler fields

 tools/lib/traceevent/event-parse-api.c   | 184 +++++++++++++++++++----
 tools/lib/traceevent/event-parse-local.h |   2 +
 tools/lib/traceevent/event-parse.c       |   4 +-
 tools/lib/traceevent/event-parse.h       |   8 +
 4 files changed, 167 insertions(+), 31 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/3] tools/lib/traceevent: Change description of few APIs
  2019-03-22 13:07 [PATCH v2 0/3] Few patches, related to libtracevent APIs Tzvetomir Stoyanov
@ 2019-03-22 13:07 ` Tzvetomir Stoyanov
  2019-03-22 13:34   ` Steven Rostedt
  2019-03-22 13:07 ` [PATCH v2 2/3] tools/lib/traceevent: Coding style fixes Tzvetomir Stoyanov
  2019-03-22 13:07 ` [PATCH v2 3/3] tools/lib/traceevent: Implement new traceevent APIs for accessing struct tep_handler fields Tzvetomir Stoyanov
  2 siblings, 1 reply; 8+ messages in thread
From: Tzvetomir Stoyanov @ 2019-03-22 13:07 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

APIs descriptions should describe the purpose of the
function, its parameters and return value. While working
on man pages implementation, I noticed mismatches in the
descriptions of few APIs. This patch changes the description
of these APIs, making them consistent with the man pages:
tep_print_num_field(), tep_print_func_field(),
tep_get_header_page_size(), tep_get_long_size(),
tep_set_long_size(), tep_get_page_size() and
tep_set_page_size().

Signed-off-by: Tzvetomir Stoyanov <tstoyanov@vmware.com>
---
 tools/lib/traceevent/event-parse-api.c | 20 ++++++++++----------
 tools/lib/traceevent/event-parse.c     |  4 ++--
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/tools/lib/traceevent/event-parse-api.c b/tools/lib/traceevent/event-parse-api.c
index d463761a58f4..3686a221e981 100644
--- a/tools/lib/traceevent/event-parse-api.c
+++ b/tools/lib/traceevent/event-parse-api.c
@@ -43,8 +43,8 @@ int tep_get_events_count(struct tep_handle *tep)
  * @flag: flag, or combination of flags to be set
  * can be any combination from enum tep_flag
  *
- * This sets a flag or mbination of flags  from enum tep_flag
-  */
+ * This sets a flag or combination of flags from enum tep_flag
+ */
 void tep_set_flag(struct tep_handle *tep, int flag)
 {
 	if(tep)
@@ -140,10 +140,10 @@ void tep_set_cpus(struct tep_handle *pevent, int cpus)
 }
 
 /**
- * tep_get_long_size - get the size of a long integer on the current machine
+ * tep_get_long_size - get the size of a long integer on the traced machine
  * @pevent: a handle to the tep_handle
  *
- * This returns the size of a long integer on the current machine
+ * This returns the size of a long integer on the traced machine
  * If @pevent is NULL, 0 is returned.
  */
 int tep_get_long_size(struct tep_handle *pevent)
@@ -154,11 +154,11 @@ int tep_get_long_size(struct tep_handle *pevent)
 }
 
 /**
- * tep_set_long_size - set the size of a long integer on the current machine
+ * tep_set_long_size - set the size of a long integer on the traced machine
  * @pevent: a handle to the tep_handle
  * @size: size, in bytes, of a long integer
  *
- * This sets the size of a long integer on the current machine
+ * This sets the size of a long integer on the traced machine
  */
 void tep_set_long_size(struct tep_handle *pevent, int long_size)
 {
@@ -167,10 +167,10 @@ void tep_set_long_size(struct tep_handle *pevent, int long_size)
 }
 
 /**
- * tep_get_page_size - get the size of a memory page on the current machine
+ * tep_get_page_size - get the size of a memory page on the traced machine
  * @pevent: a handle to the tep_handle
  *
- * This returns the size of a memory page on the current machine
+ * This returns the size of a memory page on the traced machine
  * If @pevent is NULL, 0 is returned.
  */
 int tep_get_page_size(struct tep_handle *pevent)
@@ -181,11 +181,11 @@ int tep_get_page_size(struct tep_handle *pevent)
 }
 
 /**
- * tep_set_page_size - set the size of a memory page on the current machine
+ * tep_set_page_size - set the size of a memory page on the traced machine
  * @pevent: a handle to the tep_handle
  * @_page_size: size of a memory page, in bytes
  *
- * This sets the size of a memory page on the current machine
+ * This sets the size of a memory page on the traced machine
  */
 void tep_set_page_size(struct tep_handle *pevent, int _page_size)
 {
diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 87494c7c619d..69af77896283 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -6386,7 +6386,7 @@ int tep_get_any_field_val(struct trace_seq *s, struct tep_event *event,
  * @record: The record with the field name.
  * @err: print default error if failed.
  *
- * Returns: 0 on success, -1 field not found, or 1 if buffer is full.
+ * Returns: 1 on success, -1 field not found, or 0 if buffer is full.
  */
 int tep_print_num_field(struct trace_seq *s, const char *fmt,
 			struct tep_event *event, const char *name,
@@ -6418,7 +6418,7 @@ int tep_print_num_field(struct trace_seq *s, const char *fmt,
  * @record: The record with the field name.
  * @err: print default error if failed.
  *
- * Returns: 0 on success, -1 field not found, or 1 if buffer is full.
+ * Returns: 1 on success, -1 field not found, or 0 if buffer is full.
  */
 int tep_print_func_field(struct trace_seq *s, const char *fmt,
 			 struct tep_event *event, const char *name,
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/3] tools/lib/traceevent: Coding style fixes
  2019-03-22 13:07 [PATCH v2 0/3] Few patches, related to libtracevent APIs Tzvetomir Stoyanov
  2019-03-22 13:07 ` [PATCH v2 1/3] tools/lib/traceevent: Change description of few APIs Tzvetomir Stoyanov
@ 2019-03-22 13:07 ` Tzvetomir Stoyanov
  2019-03-22 13:07 ` [PATCH v2 3/3] tools/lib/traceevent: Implement new traceevent APIs for accessing struct tep_handler fields Tzvetomir Stoyanov
  2 siblings, 0 replies; 8+ messages in thread
From: Tzvetomir Stoyanov @ 2019-03-22 13:07 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

Fixed few coding style problems in event-parse-api.c

Signed-off-by: Tzvetomir Stoyanov <tstoyanov@vmware.com>
---
 tools/lib/traceevent/event-parse-api.c | 30 +++++++++++++-------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/tools/lib/traceevent/event-parse-api.c b/tools/lib/traceevent/event-parse-api.c
index 3686a221e981..3716a9142aef 100644
--- a/tools/lib/traceevent/event-parse-api.c
+++ b/tools/lib/traceevent/event-parse-api.c
@@ -32,7 +32,7 @@ struct tep_event *tep_get_first_event(struct tep_handle *tep)
  */
 int tep_get_events_count(struct tep_handle *tep)
 {
-	if(tep)
+	if (tep)
 		return tep->nr_events;
 	return 0;
 }
@@ -47,7 +47,7 @@ int tep_get_events_count(struct tep_handle *tep)
  */
 void tep_set_flag(struct tep_handle *tep, int flag)
 {
-	if(tep)
+	if (tep)
 		tep->flags |= flag;
 }
 
@@ -108,7 +108,7 @@ tep_data2host8(struct tep_handle *pevent, unsigned long long data)
  */
 int tep_get_header_page_size(struct tep_handle *pevent)
 {
-	if(pevent)
+	if (pevent)
 		return pevent->header_page_size_size;
 	return 0;
 }
@@ -122,7 +122,7 @@ int tep_get_header_page_size(struct tep_handle *pevent)
  */
 int tep_get_cpus(struct tep_handle *pevent)
 {
-	if(pevent)
+	if (pevent)
 		return pevent->cpus;
 	return 0;
 }
@@ -135,7 +135,7 @@ int tep_get_cpus(struct tep_handle *pevent)
  */
 void tep_set_cpus(struct tep_handle *pevent, int cpus)
 {
-	if(pevent)
+	if (pevent)
 		pevent->cpus = cpus;
 }
 
@@ -148,7 +148,7 @@ void tep_set_cpus(struct tep_handle *pevent, int cpus)
  */
 int tep_get_long_size(struct tep_handle *pevent)
 {
-	if(pevent)
+	if (pevent)
 		return pevent->long_size;
 	return 0;
 }
@@ -162,7 +162,7 @@ int tep_get_long_size(struct tep_handle *pevent)
  */
 void tep_set_long_size(struct tep_handle *pevent, int long_size)
 {
-	if(pevent)
+	if (pevent)
 		pevent->long_size = long_size;
 }
 
@@ -175,7 +175,7 @@ void tep_set_long_size(struct tep_handle *pevent, int long_size)
  */
 int tep_get_page_size(struct tep_handle *pevent)
 {
-	if(pevent)
+	if (pevent)
 		return pevent->page_size;
 	return 0;
 }
@@ -189,7 +189,7 @@ int tep_get_page_size(struct tep_handle *pevent)
  */
 void tep_set_page_size(struct tep_handle *pevent, int _page_size)
 {
-	if(pevent)
+	if (pevent)
 		pevent->page_size = _page_size;
 }
 
@@ -202,7 +202,7 @@ void tep_set_page_size(struct tep_handle *pevent, int _page_size)
  */
 int tep_file_bigendian(struct tep_handle *pevent)
 {
-	if(pevent)
+	if (pevent)
 		return pevent->file_bigendian;
 	return 0;
 }
@@ -216,7 +216,7 @@ int tep_file_bigendian(struct tep_handle *pevent)
  */
 void tep_set_file_bigendian(struct tep_handle *pevent, enum tep_endian endian)
 {
-	if(pevent)
+	if (pevent)
 		pevent->file_bigendian = endian;
 }
 
@@ -229,7 +229,7 @@ void tep_set_file_bigendian(struct tep_handle *pevent, enum tep_endian endian)
  */
 int tep_is_host_bigendian(struct tep_handle *pevent)
 {
-	if(pevent)
+	if (pevent)
 		return pevent->host_bigendian;
 	return 0;
 }
@@ -243,7 +243,7 @@ int tep_is_host_bigendian(struct tep_handle *pevent)
  */
 void tep_set_host_bigendian(struct tep_handle *pevent, enum tep_endian endian)
 {
-	if(pevent)
+	if (pevent)
 		pevent->host_bigendian = endian;
 }
 
@@ -256,7 +256,7 @@ void tep_set_host_bigendian(struct tep_handle *pevent, enum tep_endian endian)
  */
 int tep_is_latency_format(struct tep_handle *pevent)
 {
-	if(pevent)
+	if (pevent)
 		return pevent->latency_format;
 	return 0;
 }
@@ -270,6 +270,6 @@ int tep_is_latency_format(struct tep_handle *pevent)
   */
 void tep_set_latency_format(struct tep_handle *pevent, int lat)
 {
-	if(pevent)
+	if (pevent)
 		pevent->latency_format = lat;
 }
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 3/3] tools/lib/traceevent: Implement new traceevent APIs for accessing struct tep_handler fields
  2019-03-22 13:07 [PATCH v2 0/3] Few patches, related to libtracevent APIs Tzvetomir Stoyanov
  2019-03-22 13:07 ` [PATCH v2 1/3] tools/lib/traceevent: Change description of few APIs Tzvetomir Stoyanov
  2019-03-22 13:07 ` [PATCH v2 2/3] tools/lib/traceevent: Coding style fixes Tzvetomir Stoyanov
@ 2019-03-22 13:07 ` Tzvetomir Stoyanov
  2019-03-22 14:25   ` Steven Rostedt
  2019-03-22 15:20   ` Steven Rostedt
  2 siblings, 2 replies; 8+ messages in thread
From: Tzvetomir Stoyanov @ 2019-03-22 13:07 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

As struct tep_handler definition is not exposed as part of libtraceevent API, its fields
cannot be accessed directly by the library users. This patch implements new APIs, which
can be used to access the struct tep_handler fields:
tep_clear_flag(), tep_test_flag(), tep_set_parsing_failures(), tep_get_parsing_failures()
tep_get_header_page_ts_size(), tep_is_old_format(), tep_set_print_raw() and tep_set_test_filters()

Signed-off-by: Tzvetomir Stoyanov <tstoyanov@vmware.com>
---
 tools/lib/traceevent/event-parse-api.c   | 134 ++++++++++++++++++++++-
 tools/lib/traceevent/event-parse-local.h |   2 +
 tools/lib/traceevent/event-parse.h       |   8 ++
 3 files changed, 140 insertions(+), 4 deletions(-)

diff --git a/tools/lib/traceevent/event-parse-api.c b/tools/lib/traceevent/event-parse-api.c
index 3716a9142aef..112a83f5caa9 100644
--- a/tools/lib/traceevent/event-parse-api.c
+++ b/tools/lib/traceevent/event-parse-api.c
@@ -8,6 +8,22 @@
 #include "event-parse-local.h"
 #include "event-utils.h"
 
+/**
+ * tep_get_event - returns the event with the given index
+ * @tep: a handle to the tep_handle
+ * @index: index of the requested event, in the range 0 .. nr_events
+ *
+ * This returns pointer to the element of the events array with the given index
+ * If @tep is NULL, or @index is not in the range 0 .. nr_events, NULL is returned.
+ */
+struct tep_event *tep_get_event(struct tep_handle *tep, int index)
+{
+	if (tep && tep->events && index < tep->nr_events)
+		return tep->events[index];
+
+	return NULL;
+}
+
 /**
  * tep_get_first_event - returns the first event in the events array
  * @tep: a handle to the tep_handle
@@ -17,10 +33,7 @@
  */
 struct tep_event *tep_get_first_event(struct tep_handle *tep)
 {
-	if (tep && tep->events)
-		return tep->events[0];
-
-	return NULL;
+	return tep_get_event(tep, 0);
 }
 
 /**
@@ -51,6 +64,35 @@ void tep_set_flag(struct tep_handle *tep, int flag)
 		tep->flags |= flag;
 }
 
+/**
+ * tep_clear_flag - clear event parser flag
+ * @tep: a handle to the tep_handle
+ * @flag: flag to be cleared
+ *
+ * This clears a tep flag
+ */
+void tep_clear_flags(struct tep_handle *tep, enum tep_flag flag)
+{
+	if (tep)
+		tep->flags &= ~flag;
+}
+
+/**
+ * tep_test_flag - check the state of event parser flag
+ * @tep: a handle to the tep_handle
+ * @flag: flag to be checked
+ *
+ * This checks the state of a tep flag.
+ * true is returned if the flag is set, false
+ * otherwise
+ */
+bool tep_test_flag(struct tep_handle *tep, enum tep_flag flag)
+{
+	if (tep)
+		return (tep->flags & flag);
+	return false;
+}
+
 unsigned short tep_data2host2(struct tep_handle *pevent, unsigned short data)
 {
 	unsigned short swap;
@@ -113,6 +155,20 @@ int tep_get_header_page_size(struct tep_handle *pevent)
 	return 0;
 }
 
+/**
+ * tep_get_header_page_ts_size - get size of the time stamp in the header page
+ * @tep: a handle to the tep_handle
+ *
+ * This returns size of the time stamp in the header page
+ * If @tep is NULL, 0 is returned.
+ */
+int tep_get_header_page_ts_size(struct tep_handle *tep)
+{
+	if (tep)
+		return tep->header_page_ts_size;
+	return 0;
+}
+
 /**
  * tep_get_cpus - get the number of CPUs
  * @pevent: a handle to the tep_handle
@@ -273,3 +329,73 @@ void tep_set_latency_format(struct tep_handle *pevent, int lat)
 	if (pevent)
 		pevent->latency_format = lat;
 }
+
+/**
+ * tep_set_parsing_failures - set parsing failures flag
+ * @tep: a handle to the tep_handle
+ * @parsing_failures: the new value of the parsing_failures flag
+ *
+ * This sets flag "parsing_failures" to the given count
+ */
+void tep_set_parsing_failures(struct tep_handle *tep, int parsing_failures)
+{
+	if (tep)
+		tep->parsing_failures = parsing_failures;
+}
+
+/**
+ * tep_get_parsing_failures - get the parsing failures flag
+ * @tep: a handle to the tep_handle
+ *
+ * This returns value of flag "parsing_failures"
+ * If @tep is NULL, 0 is returned.
+ */
+int tep_get_parsing_failures(struct tep_handle *tep)
+{
+	if (tep)
+		return tep->parsing_failures;
+	return 0;
+}
+
+/**
+ * tep_is_old_format - get if an old kernel is used
+ * @tep: a handle to the tep_handle
+ *
+ * This returns true, if an old kernel is used to generate the tracing events or
+ * false if a new kernel is used. Old kernels did not have header page info.
+ * If @pevent is NULL, false is returned.
+ */
+bool tep_is_old_format(struct tep_handle *tep)
+{
+	if (tep)
+		return !!(tep->old_format);
+	return false;
+}
+
+/**
+ * tep_set_print_raw - set a flag to force print in raw format
+ * @tep: a handle to the tep_handle
+ * @print_raw: the new value of the print_raw flag
+ *
+ * This sets a flag to force print in raw format
+ */
+void tep_set_print_raw(struct tep_handle *tep, int print_raw)
+{
+	if (tep)
+		tep->print_raw = print_raw;
+}
+
+/**
+ * tep_set_print_raw - set a flag to test a filter string
+ * @tep: a handle to the tep_handle
+ * @test_filters: the new value of the test_filters flag
+ *
+ * This sets a flag to fjust test a filter string. If this flag is set,
+ * when a filter string is added, then it will print the filters strings
+ * that were created and exit.
+ */
+void tep_set_test_filters(struct tep_handle *tep, int test_filters)
+{
+	if (tep)
+		tep->test_filters = test_filters;
+}
diff --git a/tools/lib/traceevent/event-parse-local.h b/tools/lib/traceevent/event-parse-local.h
index 35833ee32d6c..c5c8eb4c4ab7 100644
--- a/tools/lib/traceevent/event-parse-local.h
+++ b/tools/lib/traceevent/event-parse-local.h
@@ -83,6 +83,8 @@ struct tep_handle {
 	struct event_handler *handlers;
 	struct tep_function_handler *func_handlers;
 
+	int parsing_failures;
+
 	/* cache */
 	struct tep_event *last_event;
 
diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index aec48f2aea8a..4b64658334de 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -409,6 +409,8 @@ void tep_print_plugins(struct trace_seq *s,
 typedef char *(tep_func_resolver_t)(void *priv,
 				    unsigned long long *addrp, char **modp);
 void tep_set_flag(struct tep_handle *tep, int flag);
+void tep_clear_flag(struct tep_handle *tep, enum tep_flag flag);
+bool tep_check_flags(struct tep_handle *tep, enum tep_flag flags);
 
 static inline int tep_host_bigendian(void)
 {
@@ -565,6 +567,12 @@ void tep_set_host_bigendian(struct tep_handle *pevent, enum tep_endian endian);
 int tep_is_latency_format(struct tep_handle *pevent);
 void tep_set_latency_format(struct tep_handle *pevent, int lat);
 int tep_get_header_page_size(struct tep_handle *pevent);
+void tep_set_parsing_failures(struct tep_handle *tep, int parsing_failures);
+int tep_get_parsing_failures(struct tep_handle *tep);
+int tep_get_header_page_ts_size(struct tep_handle *tep);
+bool tep_is_old_format(struct tep_handle *pevent);
+void tep_set_print_raw(struct tep_handle *tep, int print_raw);
+void tep_set_test_filters(struct tep_handle *tep, int test_filters);
 
 struct tep_handle *tep_alloc(void);
 void tep_free(struct tep_handle *pevent);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/3] tools/lib/traceevent: Change description of few APIs
  2019-03-22 13:07 ` [PATCH v2 1/3] tools/lib/traceevent: Change description of few APIs Tzvetomir Stoyanov
@ 2019-03-22 13:34   ` Steven Rostedt
  0 siblings, 0 replies; 8+ messages in thread
From: Steven Rostedt @ 2019-03-22 13:34 UTC (permalink / raw)
  To: Tzvetomir Stoyanov; +Cc: linux-trace-devel

On Fri, 22 Mar 2019 15:07:40 +0200
Tzvetomir Stoyanov <tstoyanov@vmware.com> wrote:

> diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
> index 87494c7c619d..69af77896283 100644
> --- a/tools/lib/traceevent/event-parse.c
> +++ b/tools/lib/traceevent/event-parse.c
> @@ -6386,7 +6386,7 @@ int tep_get_any_field_val(struct trace_seq *s, struct tep_event *event,
>   * @record: The record with the field name.
>   * @err: print default error if failed.
>   *
> - * Returns: 0 on success, -1 field not found, or 1 if buffer is full.
> + * Returns: 1 on success, -1 field not found, or 0 if buffer is full.

Does it?

It returns trace_seq_printf() which returns:

  > 0 on succes (the length of characters written to s)
  0 if the buffer is filled
  < 0 on error.

I don't think we want to state "1" or "-1".

>   */
>  int tep_print_num_field(struct trace_seq *s, const char *fmt,
>  			struct tep_event *event, const char *name,
> @@ -6418,7 +6418,7 @@ int tep_print_num_field(struct trace_seq *s, const char *fmt,
>   * @record: The record with the field name.
>   * @err: print default error if failed.
>   *
> - * Returns: 0 on success, -1 field not found, or 1 if buffer is full.
> + * Returns: 1 on success, -1 field not found, or 0 if buffer is full.

Same here.

-- Steve

>   */
>  int tep_print_func_field(struct trace_seq *s, const char *fmt,
>  			 struct tep_event *event, const char *name,


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 3/3] tools/lib/traceevent: Implement new traceevent APIs for accessing struct tep_handler fields
  2019-03-22 13:07 ` [PATCH v2 3/3] tools/lib/traceevent: Implement new traceevent APIs for accessing struct tep_handler fields Tzvetomir Stoyanov
@ 2019-03-22 14:25   ` Steven Rostedt
  2019-03-22 15:20   ` Steven Rostedt
  1 sibling, 0 replies; 8+ messages in thread
From: Steven Rostedt @ 2019-03-22 14:25 UTC (permalink / raw)
  To: Tzvetomir Stoyanov; +Cc: linux-trace-devel

On Fri, 22 Mar 2019 15:07:42 +0200
Tzvetomir Stoyanov <tstoyanov@vmware.com> wrote:

> As struct tep_handler definition is not exposed as part of libtraceevent API, its fields
> cannot be accessed directly by the library users. This patch implements new APIs, which
> can be used to access the struct tep_handler fields:
> tep_clear_flag(), tep_test_flag(), tep_set_parsing_failures(), tep_get_parsing_failures()
> tep_get_header_page_ts_size(), tep_is_old_format(), tep_set_print_raw() and tep_set_test_filters()

Also, I want to get out of the habit of just listing what changed, and
having a more descriptive change log. It was fine for man pages, but
for adding or modifying the actual code, it needs more detail. Like:

This patch implements new APIs which can be used to access the struct
tep_handle fields:

 tep_get_event() - retrieves an event pointer at a specific index

  modify tep_get_first_event() to use tep_get_event()

 tep_clear_flag() - clears a a tep handle flag

[ BTW, you have in the patch "tep_clear_flags" let's not have it be
  plural. Otherwise we should not be an enum. ]

 tep_test_flag() - test if a given flag is set

 tep_get_header_page_ts_size() - ...

  [ Wait! we are renaming this one. Let's use the new name ]

 tep_get_header_timestamp_size() - returns the size of the timestamp
				   stored in the header.

 tep_get_cpus - returns the number of CPUs

 tep_set_parsing_failures() - internal use function for setting failures

 tep_get_parsing_failures() - Returns what was set above

 tep_is_old_format() - Returns true if data was created by an older
		       kernel with the old data format

 tep_set_print_raw() - Have the output print in the raw format

 tep_set_test_filters() - debugging utility for testing tep filters

-- Steve


> 
> Signed-off-by: Tzvetomir Stoyanov <tstoyanov@vmware.com>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 3/3] tools/lib/traceevent: Implement new traceevent APIs for accessing struct tep_handler fields
  2019-03-22 13:07 ` [PATCH v2 3/3] tools/lib/traceevent: Implement new traceevent APIs for accessing struct tep_handler fields Tzvetomir Stoyanov
  2019-03-22 14:25   ` Steven Rostedt
@ 2019-03-22 15:20   ` Steven Rostedt
  2019-03-22 15:38     ` Tzvetomir Stoyanov
  1 sibling, 1 reply; 8+ messages in thread
From: Steven Rostedt @ 2019-03-22 15:20 UTC (permalink / raw)
  To: Tzvetomir Stoyanov; +Cc: linux-trace-devel

On Fri, 22 Mar 2019 15:07:42 +0200
Tzvetomir Stoyanov <tstoyanov@vmware.com> wrote:

> +/**
> + * tep_clear_flag - clear event parser flag
> + * @tep: a handle to the tep_handle
> + * @flag: flag to be cleared
> + *
> + * This clears a tep flag
> + */
> +void tep_clear_flags(struct tep_handle *tep, enum tep_flag flag)

 void tep_clear_flag(..)

> +{
> +	if (tep)
> +		tep->flags &= ~flag;
> +}
> +
> +/**
> + * tep_test_flag - check the state of event parser flag
> + * @tep: a handle to the tep_handle
> + * @flag: flag to be checked
> + *
> + * This checks the state of a tep flag.
> + * true is returned if the flag is set, false
> + * otherwise

Let's make this a bit more formal:

 * This returns the state of the requested tep flag.
 * Returns: true if the flag is set, false otherwise.

Also, set the return in the change logs to use the full 80 characters.
There's no reason to put "otherwise" on a new line. Unless we plan on
reading this on our phones ;-)

> + */
> +bool tep_test_flag(struct tep_handle *tep, enum tep_flag flag)
> +{
> +	if (tep)
> +		return (tep->flags & flag);
> +	return false;
> +}
> +
>  unsigned short tep_data2host2(struct tep_handle *pevent, unsigned short data)
>  {
>  	unsigned short swap;
> @@ -113,6 +155,20 @@ int tep_get_header_page_size(struct tep_handle *pevent)
>  	return 0;
>  }
>  
> +/**
> + * tep_get_header_page_ts_size - get size of the time stamp in the header page
> + * @tep: a handle to the tep_handle

I already mentioned that we should use the new name.

> + *
> + * This returns size of the time stamp in the header page
> + * If @tep is NULL, 0 is returned.
> + */
> +int tep_get_header_page_ts_size(struct tep_handle *tep)
> +{
> +	if (tep)
> +		return tep->header_page_ts_size;
> +	return 0;
> +}
> +
>  /**
>   * tep_get_cpus - get the number of CPUs
>   * @pevent: a handle to the tep_handle
> @@ -273,3 +329,73 @@ void tep_set_latency_format(struct tep_handle *pevent, int lat)
>  	if (pevent)
>  		pevent->latency_format = lat;
>  }
> +
> +/**
> + * tep_set_parsing_failures - set parsing failures flag
> + * @tep: a handle to the tep_handle
> + * @parsing_failures: the new value of the parsing_failures flag
> + *
> + * This sets flag "parsing_failures" to the given count
> + */
> +void tep_set_parsing_failures(struct tep_handle *tep, int parsing_failures)
> +{
> +	if (tep)
> +		tep->parsing_failures = parsing_failures;
> +}
> +
> +/**
> + * tep_get_parsing_failures - get the parsing failures flag
> + * @tep: a handle to the tep_handle
> + *
> + * This returns value of flag "parsing_failures"
> + * If @tep is NULL, 0 is returned.
> + */
> +int tep_get_parsing_failures(struct tep_handle *tep)
> +{
> +	if (tep)
> +		return tep->parsing_failures;
> +	return 0;
> +}

Hmm, actually I think we can get rid of the parsing failures
completely, and use another mechanism to report these errors.

Or better yet, have tep_parse_event() set it internally and have:

 tep_has_failures() - returns true if tep_parse_event() failed
 tep_clear_failures() - resets so tep_has_failures() returns false


> +
> +/**
> + * tep_is_old_format - get if an old kernel is used

    - returns true if the data is from an old kernel

> + * @tep: a handle to the tep_handle
> + *
> + * This returns true, if an old kernel is used to generate the tracing events or
> + * false if a new kernel is used. Old kernels did not have header page info.
> + * If @pevent is NULL, false is returned.
> + */
> +bool tep_is_old_format(struct tep_handle *tep)
> +{
> +	if (tep)
> +		return !!(tep->old_format);
> +	return false;
> +}
> +
> +/**
> + * tep_set_print_raw - set a flag to force print in raw format
> + * @tep: a handle to the tep_handle
> + * @print_raw: the new value of the print_raw flag
> + *
> + * This sets a flag to force print in raw format
> + */
> +void tep_set_print_raw(struct tep_handle *tep, int print_raw)
> +{
> +	if (tep)
> +		tep->print_raw = print_raw;
> +}
> +
> +/**
> + * tep_set_print_raw - set a flag to test a filter string

  tep_set_test_filters - 

> + * @tep: a handle to the tep_handle
> + * @test_filters: the new value of the test_filters flag
> + *
> + * This sets a flag to fjust test a filter string. If this flag is set,

 This causes the tep_filter_add_filter_str() to simply print the filter
 instead of adding it.

Note, we should remove the "exit(0)" from tep_filter_add_filter_str()
and have the callers do the exit. A library function should not call
exit.

-- Steve


> + * when a filter string is added, then it will print the filters strings
> + * that were created and exit.
> + */
> +void tep_set_test_filters(struct tep_handle *tep, int test_filters)
> +{
> +	if (tep)
> +		tep->test_filters = test_filters;
> +}
> diff --git a/tools/lib/traceevent/event-parse-local.h b/tools/lib/traceevent/event-parse-local.h
> index 35833ee32d6c..c5c8eb4c4ab7 100644
> --- a/tools/lib/traceevent/event-parse-local.h
> +++ b/tools/lib/traceevent/event-parse-local.h
> @@ -83,6 +83,8 @@ struct tep_handle {
>  	struct event_handler *handlers;
>  	struct tep_function_handler *func_handlers;
>  
> +	int parsing_failures;
> +
>  	/* cache */
>  	struct tep_event *last_event;
>  
> diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
> index aec48f2aea8a..4b64658334de 100644
> --- a/tools/lib/traceevent/event-parse.h
> +++ b/tools/lib/traceevent/event-parse.h
> @@ -409,6 +409,8 @@ void tep_print_plugins(struct trace_seq *s,
>  typedef char *(tep_func_resolver_t)(void *priv,
>  				    unsigned long long *addrp, char **modp);
>  void tep_set_flag(struct tep_handle *tep, int flag);
> +void tep_clear_flag(struct tep_handle *tep, enum tep_flag flag);
> +bool tep_check_flags(struct tep_handle *tep, enum tep_flag flags);
>  
>  static inline int tep_host_bigendian(void)
>  {
> @@ -565,6 +567,12 @@ void tep_set_host_bigendian(struct tep_handle *pevent, enum tep_endian endian);
>  int tep_is_latency_format(struct tep_handle *pevent);
>  void tep_set_latency_format(struct tep_handle *pevent, int lat);
>  int tep_get_header_page_size(struct tep_handle *pevent);
> +void tep_set_parsing_failures(struct tep_handle *tep, int parsing_failures);
> +int tep_get_parsing_failures(struct tep_handle *tep);
> +int tep_get_header_page_ts_size(struct tep_handle *tep);
> +bool tep_is_old_format(struct tep_handle *pevent);
> +void tep_set_print_raw(struct tep_handle *tep, int print_raw);
> +void tep_set_test_filters(struct tep_handle *tep, int test_filters);
>  
>  struct tep_handle *tep_alloc(void);
>  void tep_free(struct tep_handle *pevent);


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 3/3] tools/lib/traceevent: Implement new traceevent APIs for accessing struct tep_handler fields
  2019-03-22 15:20   ` Steven Rostedt
@ 2019-03-22 15:38     ` Tzvetomir Stoyanov
  0 siblings, 0 replies; 8+ messages in thread
From: Tzvetomir Stoyanov @ 2019-03-22 15:38 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: linux-trace-devel

On Fri, Mar 22, 2019 at 5:20 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
...
> > +/**
> > + * tep_get_parsing_failures - get the parsing failures flag
> > + * @tep: a handle to the tep_handle
> > + *
> > + * This returns value of flag "parsing_failures"
> > + * If @tep is NULL, 0 is returned.
> > + */
> > +int tep_get_parsing_failures(struct tep_handle *tep)
> > +{
> > +     if (tep)
> > +             return tep->parsing_failures;
> > +     return 0;
> > +}
>
> Hmm, actually I think we can get rid of the parsing failures
> completely, and use another mechanism to report these errors.
>
> Or better yet, have tep_parse_event() set it internally and have:
>
>  tep_has_failures() - returns true if tep_parse_event() failed
>  tep_clear_failures() - resets so tep_has_failures() returns false
>
>
Ok, I'll remove tep_get_parsing_failures() from this patch. This modification
should be introduced with its own patch.

> > +
> > +/**
> > + * tep_is_old_format - get if an old kernel is used
>
>     - returns true if the data is from an old kernel
>
> > + * @tep: a handle to the tep_handle
> > + *
> > + * This returns true, if an old kernel is used to generate the tracing events or
> > + * false if a new kernel is used. Old kernels did not have header page info.
> > + * If @pevent is NULL, false is returned.
> > + */
> > +bool tep_is_old_format(struct tep_handle *tep)
> > +{
> > +     if (tep)
> > +             return !!(tep->old_format);
> > +     return false;
> > +}
> > +
> > +/**
> > + * tep_set_print_raw - set a flag to force print in raw format
> > + * @tep: a handle to the tep_handle
> > + * @print_raw: the new value of the print_raw flag
> > + *
> > + * This sets a flag to force print in raw format
> > + */
> > +void tep_set_print_raw(struct tep_handle *tep, int print_raw)
> > +{
> > +     if (tep)
> > +             tep->print_raw = print_raw;
> > +}
> > +
> > +/**
> > + * tep_set_print_raw - set a flag to test a filter string
>
>   tep_set_test_filters -
>
> > + * @tep: a handle to the tep_handle
> > + * @test_filters: the new value of the test_filters flag
> > + *
> > + * This sets a flag to fjust test a filter string. If this flag is set,
>
>  This causes the tep_filter_add_filter_str() to simply print the filter
>  instead of adding it.
>
> Note, we should remove the "exit(0)" from tep_filter_add_filter_str()
> and have the callers do the exit. A library function should not call
> exit.
>
> -- Steve
>
>
> > + * when a filter string is added, then it will print the filters strings
> > + * that were created and exit.
> > + */
> > +void tep_set_test_filters(struct tep_handle *tep, int test_filters)
> > +{
> > +     if (tep)
> > +             tep->test_filters = test_filters;
> > +}
> > diff --git a/tools/lib/traceevent/event-parse-local.h b/tools/lib/traceevent/event-parse-local.h
> > index 35833ee32d6c..c5c8eb4c4ab7 100644
> > --- a/tools/lib/traceevent/event-parse-local.h
> > +++ b/tools/lib/traceevent/event-parse-local.h
> > @@ -83,6 +83,8 @@ struct tep_handle {
> >       struct event_handler *handlers;
> >       struct tep_function_handler *func_handlers;
> >
> > +     int parsing_failures;
> > +
> >       /* cache */
> >       struct tep_event *last_event;
> >
> > diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
> > index aec48f2aea8a..4b64658334de 100644
> > --- a/tools/lib/traceevent/event-parse.h
> > +++ b/tools/lib/traceevent/event-parse.h
> > @@ -409,6 +409,8 @@ void tep_print_plugins(struct trace_seq *s,
> >  typedef char *(tep_func_resolver_t)(void *priv,
> >                                   unsigned long long *addrp, char **modp);
> >  void tep_set_flag(struct tep_handle *tep, int flag);
> > +void tep_clear_flag(struct tep_handle *tep, enum tep_flag flag);
> > +bool tep_check_flags(struct tep_handle *tep, enum tep_flag flags);
> >
> >  static inline int tep_host_bigendian(void)
> >  {
> > @@ -565,6 +567,12 @@ void tep_set_host_bigendian(struct tep_handle *pevent, enum tep_endian endian);
> >  int tep_is_latency_format(struct tep_handle *pevent);
> >  void tep_set_latency_format(struct tep_handle *pevent, int lat);
> >  int tep_get_header_page_size(struct tep_handle *pevent);
> > +void tep_set_parsing_failures(struct tep_handle *tep, int parsing_failures);
> > +int tep_get_parsing_failures(struct tep_handle *tep);
> > +int tep_get_header_page_ts_size(struct tep_handle *tep);
> > +bool tep_is_old_format(struct tep_handle *pevent);
> > +void tep_set_print_raw(struct tep_handle *tep, int print_raw);
> > +void tep_set_test_filters(struct tep_handle *tep, int test_filters);
> >
> >  struct tep_handle *tep_alloc(void);
> >  void tep_free(struct tep_handle *pevent);
>


-- 

Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-03-22 15:38 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-22 13:07 [PATCH v2 0/3] Few patches, related to libtracevent APIs Tzvetomir Stoyanov
2019-03-22 13:07 ` [PATCH v2 1/3] tools/lib/traceevent: Change description of few APIs Tzvetomir Stoyanov
2019-03-22 13:34   ` Steven Rostedt
2019-03-22 13:07 ` [PATCH v2 2/3] tools/lib/traceevent: Coding style fixes Tzvetomir Stoyanov
2019-03-22 13:07 ` [PATCH v2 3/3] tools/lib/traceevent: Implement new traceevent APIs for accessing struct tep_handler fields Tzvetomir Stoyanov
2019-03-22 14:25   ` Steven Rostedt
2019-03-22 15:20   ` Steven Rostedt
2019-03-22 15:38     ` Tzvetomir Stoyanov

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.