[PATCH 0/3] libtraceevent: Various API fixes

[PATCH 0/3] libtraceevent: Various API fixes

[Tzvetomir Stoyanov]

This patch series contains few unrelated changes in 
the library APIs. It improves the consistency of the APIs
return logic and removes a duplicated API. 

Tzvetomir Stoyanov (3):
  tools/lib/traceevent: Remove tep_data_event_from_type() API
  tools/lib/traceevent: Changed return logic of
    tep_register_event_handler() API
  tools/lib/traceevent: Changed return logic of trace_seq_printf() and
    trace_seq_vprintf() APIs

 .../libtraceevent-reg_event_handler.txt       | 12 +++++++---
 .../libtraceevent-tseq_print.txt              |  9 ++++----
 .../Documentation/libtraceevent.txt           |  1 -
 tools/lib/traceevent/event-parse.c            | 22 +++++++------------
 tools/lib/traceevent/event-parse.h            |  6 ++++-
 tools/lib/traceevent/trace-seq.c              | 17 +++++++++-----
 6 files changed, 39 insertions(+), 28 deletions(-)

-- 
2.19.1

[PATCH 1/3] tools/lib/traceevent: Remove tep_data_event_from_type() API

[Tzvetomir Stoyanov]

In order to make libtraceevent into a proper library, its API
should be straightforward. After discussion with Steven Rostedt,
we decided to remove the tep_data_event_from_type() API and to
replace it with tep_find_event(), as it does the same.

Signed-off-by: Tzvetomir Stoyanov <tstoyanov@vmware.com>
---
 tools/lib/traceevent/Documentation/libtraceevent.txt |  1 -
 tools/lib/traceevent/event-parse.c                   | 12 ------------
 tools/lib/traceevent/event-parse.h                   |  1 -
 3 files changed, 14 deletions(-)

diff --git a/tools/lib/traceevent/Documentation/libtraceevent.txt b/tools/lib/traceevent/Documentation/libtraceevent.txt
index 1100240a8a29..f2679a055c1a 100644
--- a/tools/lib/traceevent/Documentation/libtraceevent.txt
+++ b/tools/lib/traceevent/Documentation/libtraceevent.txt
@@ -63,7 +63,6 @@ Event finding:
 	struct tep_event pass:[*]*tep_find_event*(struct tep_handle pass:[*]_tep_, int _id_);
 	struct tep_event pass:[*]*tep_find_event_by_name*(struct tep_handle pass:[*]_tep_, const char pass:[*]_sys_, const char pass:[*]_name_);
 	struct tep_event pass:[*]*tep_find_event_by_record*(struct tep_handle pass:[*]_tep_, struct tep_record pass:[*]_record_);
-	struct tep_event pass:[*]*tep_data_event_from_type*(struct tep_handle pass:[*]_tep_, int _type_);
 
 Parsing of event files:
 	int *tep_parse_header_page*(struct tep_handle pass:[*]_tep_, char pass:[*]_buf_, unsigned long _size_, int _long_size_);
diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 966c4b7fd163..2970358b4c10 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -5240,18 +5240,6 @@ int tep_data_type(struct tep_handle *pevent, struct tep_record *rec)
 	return trace_parse_common_type(pevent, rec->data);
 }
 
-/**
- * tep_data_event_from_type - find the event by a given type
- * @pevent: a handle to the pevent
- * @type: the type of the event.
- *
- * This returns the event form a given @type;
- */
-struct tep_event *tep_data_event_from_type(struct tep_handle *pevent, int type)
-{
-	return tep_find_event(pevent, type);
-}
-
 /**
  * tep_data_pid - parse the PID from record
  * @pevent: a handle to the pevent
diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index 262abe3c6c9e..c6bdebf0a0ed 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -520,7 +520,6 @@ tep_find_event_by_record(struct tep_handle *pevent, struct tep_record *record);
 void tep_data_lat_fmt(struct tep_handle *pevent,
 		      struct trace_seq *s, struct tep_record *record);
 int tep_data_type(struct tep_handle *pevent, struct tep_record *rec);
-struct tep_event *tep_data_event_from_type(struct tep_handle *pevent, int type);
 int tep_data_pid(struct tep_handle *pevent, struct tep_record *rec);
 int tep_data_preempt_count(struct tep_handle *pevent, struct tep_record *rec);
 int tep_data_flags(struct tep_handle *pevent, struct tep_record *rec);
-- 
2.19.1

[PATCH 2/3] tools/lib/traceevent: Changed return logic of tep_register_event_handler() API

[Tzvetomir Stoyanov]

In order to make libtraceevent into a proper library, its API
should be straightforward. The tep_register_event_handler()
functions returns -1 in case it successfully registers the
new event handler. Such return code is used by the other library
APIs in case of an error. To unify the return logic of
tep_register_event_handler() with the other APIs, this patch
introduces enum tep_reg_handler, which is used by this function
as return value, to handle all possible successful return cases.

Signed-off-by: Tzvetomir Stoyanov <tstoyanov@vmware.com>
---
 .../libtraceevent-reg_event_handler.txt              | 12 +++++++++---
 tools/lib/traceevent/event-parse.c                   | 10 ++++++++--
 tools/lib/traceevent/event-parse.h                   |  5 +++++
 3 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/tools/lib/traceevent/Documentation/libtraceevent-reg_event_handler.txt b/tools/lib/traceevent/Documentation/libtraceevent-reg_event_handler.txt
index 1a9269039f6c..ee9aa3a4151b 100644
--- a/tools/lib/traceevent/Documentation/libtraceevent-reg_event_handler.txt
+++ b/tools/lib/traceevent/Documentation/libtraceevent-reg_event_handler.txt
@@ -12,6 +12,11 @@ SYNOPSIS
 --
 *#include <event-parse.h>*
 
+enum *tep_reg_handler* {
+	_TEP_REGISTER_SUCCESS_,
+	_TEP_REGISTER_SUCCESS_OVERWRITE_,
+};
+
 int *tep_register_event_handler*(struct tep_handle pass:[*]_tep_, int _id_, const char pass:[*]_sys_name_, const char pass:[*]_event_name_, tep_event_handler_func _func_, void pass:[*]_context_);
 int *tep_unregister_event_handler*(struct tep_handle pass:[*]tep, int id, const char pass:[*]sys_name, const char pass:[*]event_name, tep_event_handler_func func, void pass:[*]_context_);
 
@@ -48,9 +53,10 @@ _context_ is custom context, set when the custom event handler is registered.
  
 RETURN VALUE
 ------------
-The _tep_register_event_handler()_ function returns 0 in case of success. 
-If there is not  enough memory to complete the registration, 
-TEP_ERRNO__MEM_ALLOC_FAILED is returned. 
+The _tep_register_event_handler()_ function returns _TEP_REGISTER_SUCCESS_
+if the new handler is registered successfully or _TEP_REGISTER_SUCCESS_OVERWRITE_ if
+an existing handler is overwritten. If there is not  enough memory to complete 
+the registration, TEP_ERRNO__MEM_ALLOC_FAILED is returned. 
 
 The _tep_unregister_event_handler()_ function returns 0 if _func_ was removed 
 successful or, -1 if the event was not found.
diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 2970358b4c10..a5048c1b9bec 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -6624,6 +6624,12 @@ static struct tep_event *search_event(struct tep_handle *pevent, int id,
  *
  * If @id is >= 0, then it is used to find the event.
  * else @sys_name and @event_name are used.
+ *
+ * Returns:
+ *  TEP_REGISTER_SUCCESS_OVERWRITE if an existing handler is overwritten
+ *  TEP_REGISTER_SUCCESS if a new handler is registered successfully
+ *  negative TEP_ERRNO_... in case of an error
+ *
  */
 int tep_register_event_handler(struct tep_handle *pevent, int id,
 			       const char *sys_name, const char *event_name,
@@ -6641,7 +6647,7 @@ int tep_register_event_handler(struct tep_handle *pevent, int id,
 
 	event->handler = func;
 	event->context = context;
-	return 0;
+	return TEP_REGISTER_SUCCESS_OVERWRITE;
 
  not_found:
 	/* Save for later use. */
@@ -6671,7 +6677,7 @@ int tep_register_event_handler(struct tep_handle *pevent, int id,
 	pevent->handlers = handle;
 	handle->context = context;
 
-	return -1;
+	return TEP_REGISTER_SUCCESS;
 }
 
 static int handle_matches(struct event_handler *handler, int id,
diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index c6bdebf0a0ed..e5bdce43b3b4 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -484,6 +484,11 @@ int tep_print_func_field(struct trace_seq *s, const char *fmt,
 			 struct tep_event *event, const char *name,
 			 struct tep_record *record, int err);
 
+enum tep_reg_handler {
+	TEP_REGISTER_SUCCESS = 0,
+	TEP_REGISTER_SUCCESS_OVERWRITE,
+};
+
 int tep_register_event_handler(struct tep_handle *pevent, int id,
 			       const char *sys_name, const char *event_name,
 			       tep_event_handler_func func, void *context);
-- 
2.19.1

[PATCH 3/3] tools/lib/traceevent: Changed return logic of trace_seq_printf() and trace_seq_vprintf() APIs

[Tzvetomir Stoyanov]

In order to make libtraceevent into a proper library, its API
should be straightforward. The trace_seq_printf() and
trace_seq_vprintf() APIs have inconsistent returned values with
the other trace_seq_* APIs. This path changes the return logic of
trace_seq_printf() and trace_seq_vprintf() - to return the number
of printed characters, as the other trace_seq_* related APIs.

Signed-off-by: Tzvetomir Stoyanov <tstoyanov@vmware.com>
---
 .../Documentation/libtraceevent-tseq_print.txt  |  9 +++++----
 tools/lib/traceevent/trace-seq.c                | 17 ++++++++++++-----
 2 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/tools/lib/traceevent/Documentation/libtraceevent-tseq_print.txt b/tools/lib/traceevent/Documentation/libtraceevent-tseq_print.txt
index ae1aa44710e7..70e222ddbe8c 100644
--- a/tools/lib/traceevent/Documentation/libtraceevent-tseq_print.txt
+++ b/tools/lib/traceevent/Documentation/libtraceevent-tseq_print.txt
@@ -30,8 +30,9 @@ list of arguments _args_ in the trace sequence _s_.
 RETURN VALUE
 ------------
 
-Both _trace_seq_printf()_ and _trace_seq_vprintf()_ functions return non-zero 
-on success, or 0 in case of an error.
+Both _trace_seq_printf()_ and _trace_seq_vprintf()_ functions return 0 if the 
+trace oversizes the buffer's free space, the number of characters printed, or 
+a negative value in case of an error.
 
 EXAMPLE
 -------
@@ -43,7 +44,7 @@ EXAMPLE
 struct trace_seq seq;
 trace_seq_init(&seq);
 ...
-if (trace_seq_printf(&seq, "example print %d", 10) == 0) {
+if (trace_seq_printf(&seq, "example print %d", 10) <= 0) {
 	/* Failed to print in the trace sequence */
 }
 ...
@@ -51,7 +52,7 @@ void my_print(char *format, ...)
 {
 	va_list ap;
 	va_start(ap, format);
-	if (trace_seq_printf(&seq, format, ap) == 0) {
+	if (trace_seq_printf(&seq, format, ap) <= 0) {
 		/* Failed to print in the trace sequence */
 	}	
 	va_end(ap);
diff --git a/tools/lib/traceevent/trace-seq.c b/tools/lib/traceevent/trace-seq.c
index 7c5cc0a57e3c..32c610bc0538 100644
--- a/tools/lib/traceevent/trace-seq.c
+++ b/tools/lib/traceevent/trace-seq.c
@@ -114,7 +114,8 @@ static void expand_buffer(struct trace_seq *s)
  * @fmt: printf format string
  *
  * It returns 0 if the trace oversizes the buffer's free
- * space, 1 otherwise.
+ * space, the number of characters printed, or a negative
+ * value in case of an error.
  *
  * The tracer may use either sequence operations or its own
  * copy to user routines. To simplify formating of a trace
@@ -143,9 +144,10 @@ trace_seq_printf(struct trace_seq *s, const char *fmt, ...)
 		goto try_again;
 	}
 
-	s->len += ret;
+	if (ret > 0)
+		s->len += ret;
 
-	return 1;
+	return ret;
 }
 
 /**
@@ -153,6 +155,10 @@ trace_seq_printf(struct trace_seq *s, const char *fmt, ...)
  * @s: trace sequence descriptor
  * @fmt: printf format string
  *
+ * It returns 0 if the trace oversizes the buffer's free
+ * space, the number of characters printed, or a negative
+ * value in case of an error.
+ * *
  * The tracer may use either sequence operations or its own
  * copy to user routines. To simplify formating of a trace
  * trace_seq_printf is used to store strings into a special
@@ -177,9 +183,10 @@ trace_seq_vprintf(struct trace_seq *s, const char *fmt, va_list args)
 		goto try_again;
 	}
 
-	s->len += ret;
+	if (ret > 0)
+		s->len += ret;
 
-	return len;
+	return ret;
 }
 
 /**
-- 
2.19.1

Re: [PATCH 0/3] libtraceevent: Various API fixes

[Steven Rostedt]

On Fri, 23 Nov 2018 13:39:58 +0000
Tzvetomir Stoyanov <tstoyanov@vmware.com> wrote:

> This patch series contains few unrelated changes in 
> the library APIs. It improves the consistency of the APIs
> return logic and removes a duplicated API. 
> 
> Tzvetomir Stoyanov (3):
>   tools/lib/traceevent: Remove tep_data_event_from_type() API
>   tools/lib/traceevent: Changed return logic of
>     tep_register_event_handler() API
>   tools/lib/traceevent: Changed return logic of trace_seq_printf() and
>     trace_seq_vprintf() APIs
> 
>  .../libtraceevent-reg_event_handler.txt       | 12 +++++++---
>  .../libtraceevent-tseq_print.txt              |  9 ++++----
>  .../Documentation/libtraceevent.txt           |  1 -
>  tools/lib/traceevent/event-parse.c            | 22 +++++++------------
>  tools/lib/traceevent/event-parse.h            |  6 ++++-
>  tools/lib/traceevent/trace-seq.c              | 17 +++++++++-----
>  6 files changed, 39 insertions(+), 28 deletions(-)
> 

This doesn't appear to apply on any tree.

Note, if you send a patch series, it must apply on the tree you are
sending it to. It should not depend on any patch that is pending and
has not yet been applied. If it does, then that pending patch needs to
be added to the series as well.

-- Steve