linux-trace-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] tools/lib/traceevent, tools/perf: traceevent API cleanup
@ 2018-11-06 12:46 Tzvetomir Stoyanov
  2018-11-06 22:33 ` Steven Rostedt
  0 siblings, 1 reply; 4+ messages in thread
From: Tzvetomir Stoyanov @ 2018-11-06 12:46 UTC (permalink / raw)
  To: rostedt@goodmis.org; +Cc: linux-trace-devel@vger.kernel.org

In order to make libtraceevent into a proper library, its API
should be straightforward. This patch hides few API functions,
intended for internal usage only:
tep_free_event(), tep_free_format_field(), __tep_data2host2(),
__tep_data2host4() and __tep_data2host8().

Signed-off-by: Tzvetomir Stoyanov <tstoyanov@vmware.com>
---
 tools/lib/traceevent/event-parse-api.c   | 27 +++++++++++++++---------
 tools/lib/traceevent/event-parse-local.h |  8 +++++++
 tools/lib/traceevent/event-parse.c       | 13 +++++++-----
 tools/lib/traceevent/event-parse.h       | 20 +-----------------
 tools/perf/util/trace-event-read.c       |  5 +++--
 5 files changed, 37 insertions(+), 36 deletions(-)

diff --git a/tools/lib/traceevent/event-parse-api.c b/tools/lib/traceevent/event-parse-api.c
index 52188921cfe3..6a1899b378b1 100644
--- a/tools/lib/traceevent/event-parse-api.c
+++ b/tools/lib/traceevent/event-parse-api.c
@@ -51,7 +51,7 @@ void tep_set_flag(struct tep_handle *tep, int flag)
 		tep->flags |= flag;
 }
 
-unsigned short __tep_data2host2(struct tep_handle *pevent, unsigned short data)
+unsigned short tep_data2host2(struct tep_handle *pevent, unsigned short data)
 {
 	unsigned short swap;
 
@@ -64,7 +64,7 @@ unsigned short __tep_data2host2(struct tep_handle *pevent, unsigned short data)
 	return swap;
 }
 
-unsigned int __tep_data2host4(struct tep_handle *pevent, unsigned int data)
+unsigned int tep_data2host4(struct tep_handle *pevent, unsigned int data)
 {
 	unsigned int swap;
 
@@ -80,7 +80,7 @@ unsigned int __tep_data2host4(struct tep_handle *pevent, unsigned int data)
 }
 
 unsigned long long
-__tep_data2host8(struct tep_handle *pevent, unsigned long long data)
+tep_data2host8(struct tep_handle *pevent, unsigned long long data)
 {
 	unsigned long long swap;
 
@@ -140,10 +140,12 @@ 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 machine,
+ * 		       where the trace is generated
  * @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 machine,
+ * where the trace is generated
  * If @pevent is NULL, 0 is returned.
  */
 int tep_get_long_size(struct tep_handle *pevent)
@@ -154,7 +156,8 @@ 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 machine,
+ * 			where the trace is generated
  * @pevent: a handle to the tep_handle
  * @size: size, in bytes, of a long integer
  *
@@ -167,10 +170,12 @@ 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 machine,
+ * 			where the trace is generated
  * @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 machine,
+ * where the trace is generated
  * If @pevent is NULL, 0 is returned.
  */
 int tep_get_page_size(struct tep_handle *pevent)
@@ -181,11 +186,13 @@ 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 machine,
+ * 			where the trace is generated
  * @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 machine,
+ * where the trace is generated
  */
 void tep_set_page_size(struct tep_handle *pevent, int _page_size)
 {
diff --git a/tools/lib/traceevent/event-parse-local.h b/tools/lib/traceevent/event-parse-local.h
index 94746efef433..288252bc329b 100644
--- a/tools/lib/traceevent/event-parse-local.h
+++ b/tools/lib/traceevent/event-parse-local.h
@@ -89,4 +89,12 @@ struct tep_handle {
 	char *trace_clock;
 };
 
+void tep_free_event(struct tep_event *event);
+void tep_free_format_field(struct tep_format_field *field);
+
+unsigned short tep_data2host2(struct tep_handle *pevent, unsigned short data);
+unsigned int tep_data2host4(struct tep_handle *pevent, unsigned int data);
+unsigned long long
+tep_data2host8(struct tep_handle *pevent, unsigned long long data);
+
 #endif /* _PARSE_EVENTS_INT_H */
diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 73347e2ef81d..4e768357a30d 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -3343,15 +3343,18 @@ tep_find_any_field(struct tep_event *event, const char *name)
 unsigned long long tep_read_number(struct tep_handle *pevent,
 				   const void *ptr, int size)
 {
+	unsigned long long val;
+
 	switch (size) {
 	case 1:
 		return *(unsigned char *)ptr;
 	case 2:
-		return tep_data2host2(pevent, ptr);
+		return tep_data2host2(pevent, *(unsigned short *)ptr);
 	case 4:
-		return tep_data2host4(pevent, ptr);
+		return tep_data2host4(pevent, *(unsigned int *)ptr);
 	case 8:
-		return tep_data2host8(pevent, ptr);
+		memcpy(&val, (ptr), sizeof(unsigned long long));
+		return tep_data2host8(pevent, val);
 	default:
 		/* BUG! */
 		return 0;
@@ -4077,7 +4080,7 @@ static void print_str_arg(struct trace_seq *s, void *data, int size,
 			f = tep_find_any_field(event, arg->string.string);
 			arg->string.offset = f->offset;
 		}
-		str_offset = tep_data2host4(pevent, data + arg->string.offset);
+		str_offset = tep_data2host4(pevent, *(unsigned int *)(data + arg->string.offset));
 		str_offset &= 0xffff;
 		print_str_to_seq(s, format, len_arg, ((char *)data) + str_offset);
 		break;
@@ -4095,7 +4098,7 @@ static void print_str_arg(struct trace_seq *s, void *data, int size,
 			f = tep_find_any_field(event, arg->bitmask.bitmask);
 			arg->bitmask.offset = f->offset;
 		}
-		bitmask_offset = tep_data2host4(pevent, data + arg->bitmask.offset);
+		bitmask_offset = tep_data2host4(pevent, *(unsigned int *)(data + arg->bitmask.offset));
 		bitmask_size = bitmask_offset >> 16;
 		bitmask_offset &= 0xffff;
 		print_bitmask_to_seq(pevent, s, format, len_arg,
diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index 16131e006cfe..eb0d5d2471a1 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -334,7 +334,7 @@ enum tep_func_arg_type {
 enum tep_flag {
 	TEP_NSEC_OUTPUT		= 1,	/* output in NSECS */
 	TEP_DISABLE_SYS_PLUGINS	= 1 << 1,
-	TEP_DISABLE_PLUGINS	= 1 << 2,
+	TEP_DISABLE_PLUGINS	= 1 << 2
 };
 
 #define TEP_ERRORS 							      \
@@ -409,22 +409,6 @@ 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);
-unsigned short __tep_data2host2(struct tep_handle *pevent, unsigned short data);
-unsigned int __tep_data2host4(struct tep_handle *pevent, unsigned int data);
-unsigned long long
-__tep_data2host8(struct tep_handle *pevent, unsigned long long data);
-
-#define tep_data2host2(pevent, ptr)	\
-	__tep_data2host2(pevent, *(unsigned short *)(ptr))
-#define tep_data2host4(pevent, ptr)	\
-	__tep_data2host4(pevent, *(unsigned int *)(ptr))
-#define tep_data2host8(pevent, ptr)	\
-({								\
-	unsigned long long __val;				\
-								\
-	memcpy(&__val, (ptr), sizeof(unsigned long long));	\
-	__tep_data2host8(pevent, __val);			\
-})
 
 static inline int tep_host_bigendian(void)
 {
@@ -477,8 +461,6 @@ enum tep_errno tep_parse_format(struct tep_handle *pevent,
 				struct tep_event **eventp,
 				const char *buf,
 				unsigned long size, const char *sys);
-void tep_free_event(struct tep_event *event);
-void tep_free_format_field(struct tep_format_field *field);
 
 void *tep_get_field_raw(struct trace_seq *s, struct tep_event *event,
 			const char *name, struct tep_record *record,
diff --git a/tools/perf/util/trace-event-read.c b/tools/perf/util/trace-event-read.c
index cdf6de82a507..3dca624399aa 100644
--- a/tools/perf/util/trace-event-read.c
+++ b/tools/perf/util/trace-event-read.c
@@ -102,7 +102,7 @@ static unsigned int read4(struct tep_handle *pevent)
 
 	if (do_read(&data, 4) < 0)
 		return 0;
-	return __tep_data2host4(pevent, data);
+	return tep_read_number(pevent, &data, 4);
 }
 
 static unsigned long long read8(struct tep_handle *pevent)
@@ -111,7 +111,8 @@ static unsigned long long read8(struct tep_handle *pevent)
 
 	if (do_read(&data, 8) < 0)
 		return 0;
-	return __tep_data2host8(pevent, data);
+	return tep_read_number(pevent, &data, 8);
+
 }
 
 static char *read_string(void)
-- 
2.17.2

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

* Re: [PATCH] tools/lib/traceevent, tools/perf: traceevent API cleanup
  2018-11-06 12:46 [PATCH] tools/lib/traceevent, tools/perf: traceevent API cleanup Tzvetomir Stoyanov
@ 2018-11-06 22:33 ` Steven Rostedt
  2018-11-07 11:40   ` Tzvetomir Stoyanov
  0 siblings, 1 reply; 4+ messages in thread
From: Steven Rostedt @ 2018-11-06 22:33 UTC (permalink / raw)
  To: Tzvetomir Stoyanov; +Cc: linux-trace-devel@vger.kernel.org

On Tue, 6 Nov 2018 12:46:13 +0000
Tzvetomir Stoyanov <tstoyanov@vmware.com> wrote:

> In order to make libtraceevent into a proper library, its API
> should be straightforward. This patch hides few API functions,
> intended for internal usage only:
> tep_free_event(), tep_free_format_field(), __tep_data2host2(),
> __tep_data2host4() and __tep_data2host8().
> 
> Signed-off-by: Tzvetomir Stoyanov <tstoyanov@vmware.com>
> ---
>  tools/lib/traceevent/event-parse-api.c   | 27 +++++++++++++++---------
>  tools/lib/traceevent/event-parse-local.h |  8 +++++++
>  tools/lib/traceevent/event-parse.c       | 13 +++++++-----
>  tools/lib/traceevent/event-parse.h       | 20 +-----------------
>  tools/perf/util/trace-event-read.c       |  5 +++--
>  5 files changed, 37 insertions(+), 36 deletions(-)
> 
> diff --git a/tools/lib/traceevent/event-parse-api.c b/tools/lib/traceevent/event-parse-api.c
> index 52188921cfe3..6a1899b378b1 100644
> --- a/tools/lib/traceevent/event-parse-api.c
> +++ b/tools/lib/traceevent/event-parse-api.c
> @@ -51,7 +51,7 @@ void tep_set_flag(struct tep_handle *tep, int flag)
>  		tep->flags |= flag;
>  }
>  
> -unsigned short __tep_data2host2(struct tep_handle *pevent, unsigned short data)
> +unsigned short tep_data2host2(struct tep_handle *pevent, unsigned short data)
>  {
>  	unsigned short swap;
>  
> @@ -64,7 +64,7 @@ unsigned short __tep_data2host2(struct tep_handle *pevent, unsigned short data)
>  	return swap;
>  }
>  
> -unsigned int __tep_data2host4(struct tep_handle *pevent, unsigned int data)
> +unsigned int tep_data2host4(struct tep_handle *pevent, unsigned int data)
>  {
>  	unsigned int swap;
>  
> @@ -80,7 +80,7 @@ unsigned int __tep_data2host4(struct tep_handle *pevent, unsigned int data)
>  }
>  
>  unsigned long long
> -__tep_data2host8(struct tep_handle *pevent, unsigned long long data)
> +tep_data2host8(struct tep_handle *pevent, unsigned long long data)
>  {
>  	unsigned long long swap;
>  
> @@ -140,10 +140,12 @@ 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 machine,
> + * 		       where the trace is generated
>   * @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 machine,
> + * where the trace is generated
>   * If @pevent is NULL, 0 is returned.
>   */
>  int tep_get_long_size(struct tep_handle *pevent)
> @@ -154,7 +156,8 @@ 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 machine,
> + * 			where the trace is generated
>   * @pevent: a handle to the tep_handle

Why the changes to the comments in tep_set/get_long size? Looks like it
doesn't belong to this patch set. There's no mention about it in the
change log.

Did these changes get accidentally added?

>   * @size: size, in bytes, of a long integer
>   *
> @@ -167,10 +170,12 @@ 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 machine,
> + * 			where the trace is generated
>   * @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 machine,
> + * where the trace is generated
>   * If @pevent is NULL, 0 is returned.
>   */
>  int tep_get_page_size(struct tep_handle *pevent)
> @@ -181,11 +186,13 @@ 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 machine,
> + * 			where the trace is generated
>   * @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 machine,
> + * where the trace is generated

Same for tep_set/get_page_size.

>   */
>  void tep_set_page_size(struct tep_handle *pevent, int _page_size)
>  {
> diff --git a/tools/lib/traceevent/event-parse-local.h b/tools/lib/traceevent/event-parse-local.h
> index 94746efef433..288252bc329b 100644
> --- a/tools/lib/traceevent/event-parse-local.h
> +++ b/tools/lib/traceevent/event-parse-local.h
> @@ -89,4 +89,12 @@ struct tep_handle {
>  	char *trace_clock;
>  };
>  
> +void tep_free_event(struct tep_event *event);
> +void tep_free_format_field(struct tep_format_field *field);
> +
> +unsigned short tep_data2host2(struct tep_handle *pevent, unsigned short data);
> +unsigned int tep_data2host4(struct tep_handle *pevent, unsigned int data);
> +unsigned long long
> +tep_data2host8(struct tep_handle *pevent, unsigned long long data);

Please keep tep_data2host8 on one line. For protocols, I'd rather not
break them up if they break 80 characters. If you do break it up, break
it at the parameters:

unsigned long long tep_data2host*(struct tep_handle *pevent,
				unsigned long long data);

> +
>  #endif /* _PARSE_EVENTS_INT_H */
> diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
> index 73347e2ef81d..4e768357a30d 100644
> --- a/tools/lib/traceevent/event-parse.c
> +++ b/tools/lib/traceevent/event-parse.c
> @@ -3343,15 +3343,18 @@ tep_find_any_field(struct tep_event *event, const char *name)
>  unsigned long long tep_read_number(struct tep_handle *pevent,
>  				   const void *ptr, int size)
>  {
> +	unsigned long long val;
> +
>  	switch (size) {
>  	case 1:
>  		return *(unsigned char *)ptr;
>  	case 2:
> -		return tep_data2host2(pevent, ptr);
> +		return tep_data2host2(pevent, *(unsigned short *)ptr);
>  	case 4:
> -		return tep_data2host4(pevent, ptr);
> +		return tep_data2host4(pevent, *(unsigned int *)ptr);
>  	case 8:
> -		return tep_data2host8(pevent, ptr);
> +		memcpy(&val, (ptr), sizeof(unsigned long long));
> +		return tep_data2host8(pevent, val);
>  	default:
>  		/* BUG! */
>  		return 0;
> @@ -4077,7 +4080,7 @@ static void print_str_arg(struct trace_seq *s, void *data, int size,
>  			f = tep_find_any_field(event, arg->string.string);
>  			arg->string.offset = f->offset;
>  		}
> -		str_offset = tep_data2host4(pevent, data + arg->string.offset);
> +		str_offset = tep_data2host4(pevent, *(unsigned int *)(data + arg->string.offset));
>  		str_offset &= 0xffff;
>  		print_str_to_seq(s, format, len_arg, ((char *)data) + str_offset);
>  		break;
> @@ -4095,7 +4098,7 @@ static void print_str_arg(struct trace_seq *s, void *data, int size,
>  			f = tep_find_any_field(event, arg->bitmask.bitmask);
>  			arg->bitmask.offset = f->offset;
>  		}
> -		bitmask_offset = tep_data2host4(pevent, data + arg->bitmask.offset);
> +		bitmask_offset = tep_data2host4(pevent, *(unsigned int *)(data + arg->bitmask.offset));
>  		bitmask_size = bitmask_offset >> 16;
>  		bitmask_offset &= 0xffff;
>  		print_bitmask_to_seq(pevent, s, format, len_arg,
> diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
> index 16131e006cfe..eb0d5d2471a1 100644
> --- a/tools/lib/traceevent/event-parse.h
> +++ b/tools/lib/traceevent/event-parse.h
> @@ -334,7 +334,7 @@ enum tep_func_arg_type {
>  enum tep_flag {
>  	TEP_NSEC_OUTPUT		= 1,	/* output in NSECS */
>  	TEP_DISABLE_SYS_PLUGINS	= 1 << 1,
> -	TEP_DISABLE_PLUGINS	= 1 << 2,
> +	TEP_DISABLE_PLUGINS	= 1 << 2

This also looks like extra. But note, please keep the extra commas, as
it doesn't hurt and makes adding new enums easier (for diffs).

>  };
>  
>  #define TEP_ERRORS 							      \
> @@ -409,22 +409,6 @@ 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);
> -unsigned short __tep_data2host2(struct tep_handle *pevent, unsigned short data);
> -unsigned int __tep_data2host4(struct tep_handle *pevent, unsigned int data);
> -unsigned long long
> -__tep_data2host8(struct tep_handle *pevent, unsigned long long data);
> -
> -#define tep_data2host2(pevent, ptr)	\
> -	__tep_data2host2(pevent, *(unsigned short *)(ptr))
> -#define tep_data2host4(pevent, ptr)	\
> -	__tep_data2host4(pevent, *(unsigned int *)(ptr))
> -#define tep_data2host8(pevent, ptr)	\
> -({								\
> -	unsigned long long __val;				\
> -								\
> -	memcpy(&__val, (ptr), sizeof(unsigned long long));	\
> -	__tep_data2host8(pevent, __val);			\
> -})
>  
>  static inline int tep_host_bigendian(void)
>  {
> @@ -477,8 +461,6 @@ enum tep_errno tep_parse_format(struct tep_handle *pevent,
>  				struct tep_event **eventp,
>  				const char *buf,
>  				unsigned long size, const char *sys);
> -void tep_free_event(struct tep_event *event);
> -void tep_free_format_field(struct tep_format_field *field);
>  
>  void *tep_get_field_raw(struct trace_seq *s, struct tep_event *event,
>  			const char *name, struct tep_record *record,


> diff --git a/tools/perf/util/trace-event-read.c b/tools/perf/util/trace-event-read.c
> index cdf6de82a507..3dca624399aa 100644
> --- a/tools/perf/util/trace-event-read.c
> +++ b/tools/perf/util/trace-event-read.c
> @@ -102,7 +102,7 @@ static unsigned int read4(struct tep_handle *pevent)
>  
>  	if (do_read(&data, 4) < 0)
>  		return 0;
> -	return __tep_data2host4(pevent, data);
> +	return tep_read_number(pevent, &data, 4);
>  }
>  
>  static unsigned long long read8(struct tep_handle *pevent)
> @@ -111,7 +111,8 @@ static unsigned long long read8(struct tep_handle *pevent)
>  
>  	if (do_read(&data, 8) < 0)
>  		return 0;
> -	return __tep_data2host8(pevent, data);
> +	return tep_read_number(pevent, &data, 8);
> +
>  }
>  
>  static char *read_string(void)

Actually, can you break this up into two patches. One that converts
perf to the above, and state in the change log that __tep_data2host*()
are going to no longer be available as a libtraceevent API, and the
second patch that makes that so.

Thanks!

-- Steve

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

* Re: [PATCH] tools/lib/traceevent, tools/perf: traceevent API cleanup
  2018-11-06 22:33 ` Steven Rostedt
@ 2018-11-07 11:40   ` Tzvetomir Stoyanov
  0 siblings, 0 replies; 4+ messages in thread
From: Tzvetomir Stoyanov @ 2018-11-07 11:40 UTC (permalink / raw)
  To: rostedt@goodmis.org; +Cc: linux-trace-devel@vger.kernel.org

On Wed, Nov 7, 2018 at 12:33 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Tue, 6 Nov 2018 12:46:13 +0000
> Tzvetomir Stoyanov <tstoyanov@vmware.com> wrote:
>
> > In order to make libtraceevent into a proper library, its API
> > should be straightforward. This patch hides few API functions,
> > intended for internal usage only:
> > tep_free_event(), tep_free_format_field(), __tep_data2host2(),
> > __tep_data2host4() and __tep_data2host8().
> >
> > Signed-off-by: Tzvetomir Stoyanov <tstoyanov@vmware.com>
> > ---
> >  tools/lib/traceevent/event-parse-api.c   | 27 +++++++++++++++---------
> >  tools/lib/traceevent/event-parse-local.h |  8 +++++++
> >  tools/lib/traceevent/event-parse.c       | 13 +++++++-----
> >  tools/lib/traceevent/event-parse.h       | 20 +-----------------
> >  tools/perf/util/trace-event-read.c       |  5 +++--
> >  5 files changed, 37 insertions(+), 36 deletions(-)
> >
> > diff --git a/tools/lib/traceevent/event-parse-api.c b/tools/lib/traceevent/event-parse-api.c
> > index 52188921cfe3..6a1899b378b1 100644
> > --- a/tools/lib/traceevent/event-parse-api.c
> > +++ b/tools/lib/traceevent/event-parse-api.c
> > @@ -51,7 +51,7 @@ void tep_set_flag(struct tep_handle *tep, int flag)
> >               tep->flags |= flag;
> >  }
> >
> > -unsigned short __tep_data2host2(struct tep_handle *pevent, unsigned short data)
> > +unsigned short tep_data2host2(struct tep_handle *pevent, unsigned short data)
> >  {
> >       unsigned short swap;
> >
> > @@ -64,7 +64,7 @@ unsigned short __tep_data2host2(struct tep_handle *pevent, unsigned short data)
> >       return swap;
> >  }
> >
> > -unsigned int __tep_data2host4(struct tep_handle *pevent, unsigned int data)
> > +unsigned int tep_data2host4(struct tep_handle *pevent, unsigned int data)
> >  {
> >       unsigned int swap;
> >
> > @@ -80,7 +80,7 @@ unsigned int __tep_data2host4(struct tep_handle *pevent, unsigned int data)
> >  }
> >
> >  unsigned long long
> > -__tep_data2host8(struct tep_handle *pevent, unsigned long long data)
> > +tep_data2host8(struct tep_handle *pevent, unsigned long long data)
> >  {
> >       unsigned long long swap;
> >
> > @@ -140,10 +140,12 @@ 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 machine,
> > + *                  where the trace is generated
> >   * @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 machine,
> > + * where the trace is generated
> >   * If @pevent is NULL, 0 is returned.
> >   */
> >  int tep_get_long_size(struct tep_handle *pevent)
> > @@ -154,7 +156,8 @@ 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 machine,
> > + *                   where the trace is generated
> >   * @pevent: a handle to the tep_handle
>
> Why the changes to the comments in tep_set/get_long size? Looks like it
> doesn't belong to this patch set. There's no mention about it in the
> change log.
>
> Did these changes get accidentally added?
>

Looks like accidentally change, it should be part of the patch which implements
tep_set_long_size() / tep_get_long_size() man page. Going to move it there.

> >   * @size: size, in bytes, of a long integer
> >   *
> > @@ -167,10 +170,12 @@ 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 machine,
> > + *                   where the trace is generated
> >   * @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 machine,
> > + * where the trace is generated
> >   * If @pevent is NULL, 0 is returned.
> >   */
> >  int tep_get_page_size(struct tep_handle *pevent)
> > @@ -181,11 +186,13 @@ 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 machine,
> > + *                   where the trace is generated
> >   * @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 machine,
> > + * where the trace is generated
>
> Same for tep_set/get_page_size.
>
> >   */
> >  void tep_set_page_size(struct tep_handle *pevent, int _page_size)
> >  {
> > diff --git a/tools/lib/traceevent/event-parse-local.h b/tools/lib/traceevent/event-parse-local.h
> > index 94746efef433..288252bc329b 100644
> > --- a/tools/lib/traceevent/event-parse-local.h
> > +++ b/tools/lib/traceevent/event-parse-local.h
> > @@ -89,4 +89,12 @@ struct tep_handle {
> >       char *trace_clock;
> >  };
> >
> > +void tep_free_event(struct tep_event *event);
> > +void tep_free_format_field(struct tep_format_field *field);
> > +
> > +unsigned short tep_data2host2(struct tep_handle *pevent, unsigned short data);
> > +unsigned int tep_data2host4(struct tep_handle *pevent, unsigned int data);
> > +unsigned long long
> > +tep_data2host8(struct tep_handle *pevent, unsigned long long data);
>
> Please keep tep_data2host8 on one line. For protocols, I'd rather not
> break them up if they break 80 characters. If you do break it up, break
> it at the parameters:
>
> unsigned long long tep_data2host*(struct tep_handle *pevent,
>                                 unsigned long long data);
>

ok, will revert it.

> > +
> >  #endif /* _PARSE_EVENTS_INT_H */
> > diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
> > index 73347e2ef81d..4e768357a30d 100644
> > --- a/tools/lib/traceevent/event-parse.c
> > +++ b/tools/lib/traceevent/event-parse.c
> > @@ -3343,15 +3343,18 @@ tep_find_any_field(struct tep_event *event, const char *name)
> >  unsigned long long tep_read_number(struct tep_handle *pevent,
> >                                  const void *ptr, int size)
> >  {
> > +     unsigned long long val;
> > +
> >       switch (size) {
> >       case 1:
> >               return *(unsigned char *)ptr;
> >       case 2:
> > -             return tep_data2host2(pevent, ptr);
> > +             return tep_data2host2(pevent, *(unsigned short *)ptr);
> >       case 4:
> > -             return tep_data2host4(pevent, ptr);
> > +             return tep_data2host4(pevent, *(unsigned int *)ptr);
> >       case 8:
> > -             return tep_data2host8(pevent, ptr);
> > +             memcpy(&val, (ptr), sizeof(unsigned long long));
> > +             return tep_data2host8(pevent, val);
> >       default:
> >               /* BUG! */
> >               return 0;
> > @@ -4077,7 +4080,7 @@ static void print_str_arg(struct trace_seq *s, void *data, int size,
> >                       f = tep_find_any_field(event, arg->string.string);
> >                       arg->string.offset = f->offset;
> >               }
> > -             str_offset = tep_data2host4(pevent, data + arg->string.offset);
> > +             str_offset = tep_data2host4(pevent, *(unsigned int *)(data + arg->string.offset));
> >               str_offset &= 0xffff;
> >               print_str_to_seq(s, format, len_arg, ((char *)data) + str_offset);
> >               break;
> > @@ -4095,7 +4098,7 @@ static void print_str_arg(struct trace_seq *s, void *data, int size,
> >                       f = tep_find_any_field(event, arg->bitmask.bitmask);
> >                       arg->bitmask.offset = f->offset;
> >               }
> > -             bitmask_offset = tep_data2host4(pevent, data + arg->bitmask.offset);
> > +             bitmask_offset = tep_data2host4(pevent, *(unsigned int *)(data + arg->bitmask.offset));
> >               bitmask_size = bitmask_offset >> 16;
> >               bitmask_offset &= 0xffff;
> >               print_bitmask_to_seq(pevent, s, format, len_arg,
> > diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
> > index 16131e006cfe..eb0d5d2471a1 100644
> > --- a/tools/lib/traceevent/event-parse.h
> > +++ b/tools/lib/traceevent/event-parse.h
> > @@ -334,7 +334,7 @@ enum tep_func_arg_type {
> >  enum tep_flag {
> >       TEP_NSEC_OUTPUT         = 1,    /* output in NSECS */
> >       TEP_DISABLE_SYS_PLUGINS = 1 << 1,
> > -     TEP_DISABLE_PLUGINS     = 1 << 2,
> > +     TEP_DISABLE_PLUGINS     = 1 << 2
>
> This also looks like extra. But note, please keep the extra commas, as
> it doesn't hurt and makes adding new enums easier (for diffs).
>

ok, going to revert it too.

> >  };
> >
> >  #define TEP_ERRORS                                                         \
> > @@ -409,22 +409,6 @@ 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);
> > -unsigned short __tep_data2host2(struct tep_handle *pevent, unsigned short data);
> > -unsigned int __tep_data2host4(struct tep_handle *pevent, unsigned int data);
> > -unsigned long long
> > -__tep_data2host8(struct tep_handle *pevent, unsigned long long data);
> > -
> > -#define tep_data2host2(pevent, ptr)  \
> > -     __tep_data2host2(pevent, *(unsigned short *)(ptr))
> > -#define tep_data2host4(pevent, ptr)  \
> > -     __tep_data2host4(pevent, *(unsigned int *)(ptr))
> > -#define tep_data2host8(pevent, ptr)  \
> > -({                                                           \
> > -     unsigned long long __val;                               \
> > -                                                             \
> > -     memcpy(&__val, (ptr), sizeof(unsigned long long));      \
> > -     __tep_data2host8(pevent, __val);                        \
> > -})
> >
> >  static inline int tep_host_bigendian(void)
> >  {
> > @@ -477,8 +461,6 @@ enum tep_errno tep_parse_format(struct tep_handle *pevent,
> >                               struct tep_event **eventp,
> >                               const char *buf,
> >                               unsigned long size, const char *sys);
> > -void tep_free_event(struct tep_event *event);
> > -void tep_free_format_field(struct tep_format_field *field);
> >
> >  void *tep_get_field_raw(struct trace_seq *s, struct tep_event *event,
> >                       const char *name, struct tep_record *record,
>
>
> > diff --git a/tools/perf/util/trace-event-read.c b/tools/perf/util/trace-event-read.c
> > index cdf6de82a507..3dca624399aa 100644
> > --- a/tools/perf/util/trace-event-read.c
> > +++ b/tools/perf/util/trace-event-read.c
> > @@ -102,7 +102,7 @@ static unsigned int read4(struct tep_handle *pevent)
> >
> >       if (do_read(&data, 4) < 0)
> >               return 0;
> > -     return __tep_data2host4(pevent, data);
> > +     return tep_read_number(pevent, &data, 4);
> >  }
> >
> >  static unsigned long long read8(struct tep_handle *pevent)
> > @@ -111,7 +111,8 @@ static unsigned long long read8(struct tep_handle *pevent)
> >
> >       if (do_read(&data, 8) < 0)
> >               return 0;
> > -     return __tep_data2host8(pevent, data);
> > +     return tep_read_number(pevent, &data, 8);
> > +
> >  }
> >
> >  static char *read_string(void)
>
> Actually, can you break this up into two patches. One that convertsko
> perf to the above, and state in the change log that __tep_data2host*()
> are going to no longer be available as a libtraceevent API, and the
> second patch that makes that so.
>
> Thanks!
>
> -- Steve

I'll send v2 addressing the comments.

--
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center

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

* [PATCH] tools/lib/traceevent, tools/perf: traceevent API cleanup
@ 2018-11-09 12:57 Tzvetomir Stoyanov
  0 siblings, 0 replies; 4+ messages in thread
From: Tzvetomir Stoyanov @ 2018-11-09 12:57 UTC (permalink / raw)
  To: rostedt@goodmis.org; +Cc: linux-trace-devel@vger.kernel.org

In order to make libtraceevent into a proper library, its API
should be straightforward. This patch hides few API functions,
intended for internal usage only:
tep_free_event(), tep_free_format_field(), __tep_data2host2(),
__tep_data2host4() and __tep_data2host8().
The patch also alignes the libtraceevent summary man page with
these API changes.

Signed-off-by: Tzvetomir Stoyanov <tstoyanov@vmware.com>
---
 .../Documentation/libtraceevent.txt           | 125 +++++++++---------
 tools/lib/traceevent/event-parse-api.c        |   6 +-
 tools/lib/traceevent/event-parse-local.h      |   7 +
 tools/lib/traceevent/event-parse.c            |  13 +-
 tools/lib/traceevent/event-parse.h            |  18 ---
 5 files changed, 79 insertions(+), 90 deletions(-)

diff --git a/tools/lib/traceevent/Documentation/libtraceevent.txt b/tools/lib/traceevent/Documentation/libtraceevent.txt
index bc3d77934ab8..e2b4ca615809 100644
--- a/tools/lib/traceevent/Documentation/libtraceevent.txt
+++ b/tools/lib/traceevent/Documentation/libtraceevent.txt
@@ -13,38 +13,34 @@ SYNOPSIS
 
 Management of tep handler data structure and access of its members:
 	struct tep_handle pass:[*]*tep_alloc*(void);
-	void *tep_free*(struct tep_handle pass:[*]_pevent_);
-	void *tep_ref*(struct tep_handle pass:[*]_pevent_);
-	void *tep_unref*(struct tep_handle pass:[*]_pevent_);
-	int *tep_ref_get*(struct tep_handle pass:[*]_pevent_);
+	void *tep_free*(struct tep_handle pass:[*]_tep_);
+	void *tep_ref*(struct tep_handle pass:[*]_tep_);
+	void *tep_unref*(struct tep_handle pass:[*]_tep_);
+	int *tep_ref_get*(struct tep_handle pass:[*]_tep_);
 	void *tep_set_flag*(struct tep_handle pass:[*]_tep_, int _flag_);
-	int *tep_get_cpus*(struct tep_handle pass:[*]_pevent_);
-	void *tep_set_cpus*(struct tep_handle pass:[*]_pevent_, int _cpus_);
-	int *tep_get_long_size*(strucqt tep_handle pass:[*]_pevent_);
-	void *tep_set_long_size*(struct tep_handle pass:[*]_pevent_, int _long_size_);
-	int *tep_get_page_size*(struct tep_handle pass:[*]_pevent_);
-	void *tep_set_page_size*(struct tep_handle pass:[*]_pevent_, int _page_size_);
-	int *tep_is_file_bigendian*(struct tep_handle pass:[*]_pevent_);
-	void *tep_set_file_bigendian*(struct tep_handle pass:[*]_pevent_, enum tep_endian _endian_);
-	int *tep_is_host_bigendian*(struct tep_handle pass:[*]_pevent_);
-	void *tep_set_host_bigendian*(struct tep_handle pass:[*]_pevent_, enum tep_endian _endian_);
-	int *tep_is_latency_format*(struct tep_handle pass:[*]_pevent_);
-	void *tep_set_latency_format*(struct tep_handle pass:[*]_pevent_, int _lat_);
-	int *tep_get_header_page_size*(struct tep_handle pass:[*]_pevent_);
-	int *tep_strerror*(struct tep_handle pass:[*]_pevent_, enum tep_errno _errnum_, char pass:[*]_buf_, size_t _buflen_);
+	int *tep_get_cpus*(struct tep_handle pass:[*]_tep_);
+	void *tep_set_cpus*(struct tep_handle pass:[*]_tep_, int _cpus_);
+	int *tep_get_long_size*(strucqt tep_handle pass:[*]_tep_);
+	void *tep_set_long_size*(struct tep_handle pass:[*]_tep_, int _long_size_);
+	int *tep_get_page_size*(struct tep_handle pass:[*]_tep_);
+	void *tep_set_page_size*(struct tep_handle pass:[*]_tep_, int _page_size_);
+	int *tep_is_latency_format*(struct tep_handle pass:[*]_tep_);
+	void *tep_set_latency_format*(struct tep_handle pass:[*]_tep_, int _lat_);
+	int *tep_get_header_page_size*(struct tep_handle pass:[*]_tep_);
+	int *tep_strerror*(struct tep_handle pass:[*]_tep_, enum tep_errno _errnum_, char pass:[*]_buf_, size_t _buflen_);
 
 Register / unregister APIs:
-	int *tep_register_trace_clock*(struct tep_handle pass:[*]_pevent_, const char pass:[*]_trace_clock_);
-	int *tep_register_function*(struct tep_handle pass:[*]_pevent_, char pass:[*]_name_, unsigned long long _addr_, char pass:[*]_mod_);
-	int *tep_register_event_handler*(struct tep_handle pass:[*]_pevent_, 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:[*]pevent, int id, const char pass:[*]sys_name, const char pass:[*]event_name, tep_event_handler_func func, void pass:[*]_context_);
-	int *tep_register_print_string*(struct tep_handle pass:[*]_pevent_, const char pass:[*]_fmt_, unsigned long long _addr_);
-	int *tep_register_print_function*(struct tep_handle pass:[*]_pevent_, tep_func_handler _func_, enum tep_func_arg_type _ret_type_, char pass:[*]_name_, _..._);
-	int *tep_unregister_print_function*(struct tep_handle pass:[*]_pevent_, tep_func_handler _func_, char pass:[*]_name_);
+	int *tep_register_trace_clock*(struct tep_handle pass:[*]_tep_, const char pass:[*]_trace_clock_);
+	int *tep_register_function*(struct tep_handle pass:[*]_tep_, char pass:[*]_name_, unsigned long long _addr_, char pass:[*]_mod_);
+	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_);
+	int *tep_register_print_string*(struct tep_handle pass:[*]_tep_, const char pass:[*]_fmt_, unsigned long long _addr_);
+	int *tep_register_print_function*(struct tep_handle pass:[*]_tep_, tep_func_handler _func_, enum tep_func_arg_type _ret_type_, char pass:[*]_name_, _..._);
+	int *tep_unregister_print_function*(struct tep_handle pass:[*]_tep_, tep_func_handler _func_, char pass:[*]_name_);
 
 Plugins management: 
-	struct tep_plugin_list pass:[*]*tep_load_plugins*(struct tep_handle pass:[*]_pevent_);
-	void *tep_unload_plugins*(struct tep_plugin_list pass:[*]_plugin_list_, struct tep_handle pass:[*]_pevent_);
+	struct tep_plugin_list pass:[*]*tep_load_plugins*(struct tep_handle pass:[*]_tep_);
+	void *tep_unload_plugins*(struct tep_plugin_list pass:[*]_plugin_list_, struct tep_handle pass:[*]_tep_);
 	char pass:[*]pass:[*]*tep_plugin_list_options*(void);
 	void *tep_plugin_free_options_list*(char pass:[*]pass:[*]_list_);
 	int *tep_plugin_add_options*(const char pass:[*]_name_, struct tep_plugin_option pass:[*]_options_);
@@ -54,40 +50,40 @@ Plugins management:
 Event related APIs:
 	struct tep_event pass:[*]*tep_get_first_event*(struct tep_handle pass:[*]_tep_);
 	int *tep_get_events_count*(struct tep_handle pass:[*]_tep_);
-	void *tep_free_event*(struct tep_event pass:[*]_event_);
-	struct tep_event pass:[*]pass:[*]*tep_list_events*(struct tep_handle pass:[*]_pevent_, enum tep_event_sort_type _sort_type_);
+	struct tep_event pass:[*]pass:[*]*tep_list_events*(struct tep_handle pass:[*]_tep_, enum tep_event_sort_type _sort_type_);
 	void *tep_event_info*(struct trace_seq pass:[*]_s_, struct tep_event pass:[*]_event_, struct tep_record pass:[*]_record_);
 
 Event printing:
-	void *tep_print_event_task*(struct tep_handle pass:[*]_pevent_, struct trace_seq pass:[*]_s_, struct tep_event pass:[*]_event_, struct tep_record pass:[*]_record_);
-	void *tep_print_event_time*(struct tep_handle pass:[*]_pevent_, struct trace_seq pass:[*]_s_, struct tep_event pass:[*]_event_, struct tep_record pass:[*]record, bool _use_trace_clock_);
-	void *tep_print_event_data*(struct tep_handle pass:[*]_pevent_, struct trace_seq pass:[*]_s_, struct tep_event pass:[*]_event_, struct tep_record pass:[*]_record_);
-	void *tep_print_event*(struct tep_handle pass:[*]_pevent_, struct trace_seq pass:[*]_s_, struct tep_record pass:[*]_record_, bool _use_trace_clock_);
+	void *tep_print_event_task*(struct tep_handle pass:[*]_tep_, struct trace_seq pass:[*]_s_, struct tep_event pass:[*]_event_, struct tep_record pass:[*]_record_);
+	void *tep_print_event_time*(struct tep_handle pass:[*]_tep_, struct trace_seq pass:[*]_s_, struct tep_event pass:[*]_event_, struct tep_record pass:[*]record, bool _use_trace_clock_);
+	void *tep_print_event_data*(struct tep_handle pass:[*]_tep_, struct trace_seq pass:[*]_s_, struct tep_event pass:[*]_event_, struct tep_record pass:[*]_record_);
+	void *tep_print_event*(struct tep_handle pass:[*]_tep_, struct trace_seq pass:[*]_s_, struct tep_record pass:[*]_record_, bool _use_trace_clock_);
 
 Event finding:
-	struct tep_event pass:[*]*tep_find_event*(struct tep_handle pass:[*]_pevent_, int _id_);
-	struct tep_event pass:[*]*tep_find_event_by_name*(struct tep_handle pass:[*]_pevent_, const char pass:[*]_sys_, const char pass:[*]_name_);
-	struct tep_event pass:[*]*tep_find_event_by_record*(struct tep_handle pass:[*]_pevent_, struct tep_record pass:[*]_record_);
-	struct tep_event pass:[*]*tep_data_event_from_type*(struct tep_handle pass:[*]_pevent_, int _type_);
+	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:[*]_pevent_, char pass:[*]_buf_, unsigned long _size_, int _long_size_);
-	enum *tep_errno tep_parse_event*(struct tep_handle pass:[*]_pevent_, const char pass:[*]_buf_, unsigned long _size_, const char pass:[*]_sys_);
-	enum *tep_errno tep_parse_format*(struct tep_handle pass:[*]_pevent_, struct tep_event pass:[*]pass:[*]_eventp_, const char pass:[*]_buf_, unsigned long _size_, const char pass:[*]_sys_);
+	int *tep_parse_header_page*(struct tep_handle pass:[*]_tep_, char pass:[*]_buf_, unsigned long _size_, int _long_size_);
+	enum tep_errno *tep_parse_event*(struct tep_handle pass:[*]_tep_, const char pass:[*]_buf_, unsigned long _size_, const char pass:[*]_sys_);
+	enum tep_errno *tep_parse_format*(struct tep_handle pass:[*]_tep_, struct tep_event pass:[*]pass:[*]_eventp_, const char pass:[*]_buf_, unsigned long _size_, const char pass:[*]_sys_);
 
 APIs related to fields from event's format files:
-	void *tep_free_format_field*(struct tep_format_field pass:[*]_field_);
 	struct tep_format_field pass:[*]pass:[*]*tep_event_common_fields*(struct tep_event pass:[*]_event_);
 	struct tep_format_field pass:[*]pass:[*]*tep_event_fields*(struct tep_event pass:[*]_event_);
 	void pass:[*]*tep_get_field_raw*(struct trace_seq pass:[*]_s_, struct tep_event pass:[*]_event_, const char pass:[*]_name_, struct tep_record pass:[*]_record_, int pass:[*]_len_, int _err_);
 	int *tep_get_field_val*(struct trace_seq pass:[*]_s_, struct tep_event pass:[*]_event_, const char pass:[*]_name_, struct tep_record pass:[*]_record_, unsigned long long pass:[*]_val_, int _err_);
 	int *tep_get_common_field_val*(struct trace_seq pass:[*]_s_, struct tep_event pass:[*]_event_, const char pass:[*]_name_, struct tep_record pass:[*]_record_, unsigned long long pass:[*]_val_, int _err_);
 	int *tep_get_any_field_val*(struct trace_seq pass:[*]_s_, struct tep_event pass:[*]_event_, const char pass:[*]_name_, struct tep_record pass:[*]_record_, unsigned long long pass:[*]_val_, int _err_);	
+	int *tep_read_number_field*(struct tep_format_field pass:[*]_field_, const void pass:[*]_data_, unsigned long long pass:[*]_value_);
+	
+Event fileds printing:
 	void *tep_print_field*(struct trace_seq pass:[*]_s_, void pass:[*]_data_, struct tep_format_field pass:[*]_field_);
 	void *tep_print_fields*(struct trace_seq pass:[*]_s_, void pass:[*]_data_, int _size_, struct tep_event pass:[*]_event_);
 	int *tep_print_num_field*(struct trace_seq pass:[*]_s_, const char pass:[*]_fmt_, struct tep_event pass:[*]_event_, const char pass:[*]_name_, struct tep_record pass:[*]_record_, int _err_);
 	int *tep_print_func_field*(struct trace_seq pass:[*]_s_, const char pass:[*]_fmt_, struct tep_event pass:[*]_event_, const char pass:[*]_name_, struct tep_record pass:[*]_record_, int _err_);
-	int *tep_read_number_field*(struct tep_format_field pass:[*]_field_, const void pass:[*]_data_, unsigned long long pass:[*]_value_);
 	
 Event fileds finding:
 	struct tep_format_field pass:[*]*tep_find_common_field*(struct tep_event pass:[*]_event_, const char pass:[*]_name_);
@@ -95,13 +91,13 @@ Event fileds finding:
 	struct tep_format_field pass:[*]*tep_find_any_field*(struct tep_event pass:[*]_event_, const char pass:[*]_name_);
 
 Functions resolver:
-	int *tep_set_function_resolver*(struct tep_handle pass:[*]_pevent_, tep_func_resolver_t pass:[*]_func_, void pass:[*]_priv_);
-	void *tep_reset_function_resolver*(struct tep_handle pass:[*]_pevent_);
-	const char pass:[*]*tep_find_function*(struct tep_handle pass:[*]_pevent_, unsigned long long _addr_);
-	unsigned long long *tep_find_function_address*(struct tep_handle pass:[*]_pevent_, unsigned long long _addr_);
+	int *tep_set_function_resolver*(struct tep_handle pass:[*]_tep_, tep_func_resolver_t pass:[*]_func_, void pass:[*]_priv_);
+	void *tep_reset_function_resolver*(struct tep_handle pass:[*]_tep_);
+	const char pass:[*]*tep_find_function*(struct tep_handle pass:[*]_tep_, unsigned long long _addr_);
+	unsigned long long *tep_find_function_address*(struct tep_handle pass:[*]_tep_, unsigned long long _addr_);
 
 Filter management:
-	struct tep_event_filter pass:[*]*tep_filter_alloc*(struct tep_handle pass:[*]_pevent_);
+	struct tep_event_filter pass:[*]*tep_filter_alloc*(struct tep_handle pass:[*]_tep_);
 	enum tep_errno *tep_filter_add_filter_str*(struct tep_event_filter pass:[*]_filter_, const char pass:[*]_filter_str_);
 	enum tep_errno *tep_filter_match*(struct tep_event_filter pass:[*]_filter_, struct tep_record pass:[*]_record_);
 	int *tep_filter_strerror*(struct tep_event_filter pass:[*]_filter_, enum tep_errno _err_, char pass:[*]buf, size_t _buflen_);
@@ -117,25 +113,26 @@ Filter management:
 	int *tep_filter_compare*(struct tep_event_filter pass:[*]_filter1_, struct tep_event_filter pass:[*]_filter2_);
 
 Parsing various data from the records:
-	void *tep_data_lat_fmt*(struct tep_handle pass:[*]_pevent_, struct trace_seq pass:[*]_s_, struct tep_record pass:[*]_record_);
-	int *tep_data_type*(struct tep_handle pass:[*]_pevent_, struct tep_record pass:[*]_rec_);
-	int *tep_data_pid*(struct tep_handle pass:[*]_pevent_, struct tep_record pass:[*]_rec_);
-	int *tep_data_preempt_count*(struct tep_handle pass:[*]_pevent_, struct tep_record pass:[*]_rec_);
-	int *tep_data_flags*(struct tep_handle pass:[*]_pevent_, struct tep_record pass:[*]_rec_);
-	const char pass:[*]*tep_data_comm_from_pid*(struct tep_handle pass:[*]_pevent_, int _pid_);
-	struct cmdline pass:[*]*tep_data_pid_from_comm*(struct tep_handle pass:[*]_pevent_, const char pass:[*]_comm_, struct cmdline pass:[*]_next_);
+	void *tep_data_lat_fmt*(struct tep_handle pass:[*]_tep_, struct trace_seq pass:[*]_s_, struct tep_record pass:[*]_record_);
+	int *tep_data_type*(struct tep_handle pass:[*]_tep_, struct tep_record pass:[*]_rec_);
+	int *tep_data_pid*(struct tep_handle pass:[*]_tep_, struct tep_record pass:[*]_rec_);
+	int *tep_data_preempt_count*(struct tep_handle pass:[*]_tep_, struct tep_record pass:[*]_rec_);
+	int *tep_data_flags*(struct tep_handle pass:[*]_tep_, struct tep_record pass:[*]_rec_);
 	
 Command and task related APIs:
-	int *tep_register_comm*(struct tep_handle pass:[*]_pevent_, const char pass:[*]_comm_, int _pid_);
-	int *tep_pid_is_registered*(struct tep_handle pass:[*]_pevent_, int _pid_);
-	int *tep_cmdline_pid*(struct tep_handle pass:[*]_pevent_, struct cmdline pass:[*]_cmdline_);
+	const char pass:[*]*tep_data_comm_from_pid*(struct tep_handle pass:[*]_tep_, int _pid_);
+	struct cmdline pass:[*]*tep_data_pid_from_comm*(struct tep_handle pass:[*]_tep_, const char pass:[*]_comm_, struct cmdline pass:[*]_next_);
+	int *tep_register_comm*(struct tep_handle pass:[*]_tep_, const char pass:[*]_comm_, int _pid_);
+	int *tep_pid_is_registered*(struct tep_handle pass:[*]_tep_, int _pid_);
+	int *tep_cmdline_pid*(struct tep_handle pass:[*]_tep_, struct cmdline pass:[*]_cmdline_);
 
 Endian related APIs:
-	unsigned short *tep_data2host2*(struct tep_handle pass:[*]_pevent_, unsigned short _data_);
-	unsigned int *tep_data2host4*(struct tep_handle pass:[*]_pevent_, unsigned int _data_);
-	unsigned long long *tep_data2host8*(struct tep_handle pass:[*]_pevent_, unsigned long long _data_);
 	int *tep_host_bigendian*(void);
-	unsigned long long *tep_read_number*(struct tep_handle pass:[*]_pevent_, const void pass:[*]_ptr_, int _size_);
+	unsigned long long *tep_read_number*(struct tep_handle pass:[*]_tep_, const void pass:[*]_ptr_, int _size_);
+	int *tep_is_file_bigendian*(struct tep_handle pass:[*]_tep_);
+	void *tep_set_file_bigendian*(struct tep_handle pass:[*]_tep_, enum tep_endian _endian_);
+	int *tep_is_host_bigendian*(struct tep_handle pass:[*]_tep_);
+	void *tep_set_host_bigendian*(struct tep_handle pass:[*]_tep_, enum tep_endian _endian_);
 
 Trace sequences:
 *#include <trace-seq.h>*
@@ -168,13 +165,13 @@ FILES
 -----
 [verse]
 --
-event-parse.h
+*event-parse.h*
 	Header file to include in order to have access to the library APIs.
-trace-seq.h
+*trace-seq.h*
 	Header file to include in order to have access to trace sequences related APIs.
 	Trace sequences are used to allow a function to call several other functions 
 	to create a string of data to use.
--ltraceevent
+*-ltraceevent*
 	Linker switch to add when building a program that uses the library.
 --
 
diff --git a/tools/lib/traceevent/event-parse-api.c b/tools/lib/traceevent/event-parse-api.c
index 52188921cfe3..bdefe3c235ad 100644
--- a/tools/lib/traceevent/event-parse-api.c
+++ b/tools/lib/traceevent/event-parse-api.c
@@ -51,7 +51,7 @@ void tep_set_flag(struct tep_handle *tep, int flag)
 		tep->flags |= flag;
 }
 
-unsigned short __tep_data2host2(struct tep_handle *pevent, unsigned short data)
+unsigned short tep_data2host2(struct tep_handle *pevent, unsigned short data)
 {
 	unsigned short swap;
 
@@ -64,7 +64,7 @@ unsigned short __tep_data2host2(struct tep_handle *pevent, unsigned short data)
 	return swap;
 }
 
-unsigned int __tep_data2host4(struct tep_handle *pevent, unsigned int data)
+unsigned int tep_data2host4(struct tep_handle *pevent, unsigned int data)
 {
 	unsigned int swap;
 
@@ -80,7 +80,7 @@ unsigned int __tep_data2host4(struct tep_handle *pevent, unsigned int data)
 }
 
 unsigned long long
-__tep_data2host8(struct tep_handle *pevent, unsigned long long data)
+tep_data2host8(struct tep_handle *pevent, unsigned long long data)
 {
 	unsigned long long swap;
 
diff --git a/tools/lib/traceevent/event-parse-local.h b/tools/lib/traceevent/event-parse-local.h
index 94746efef433..9a092dd4a86d 100644
--- a/tools/lib/traceevent/event-parse-local.h
+++ b/tools/lib/traceevent/event-parse-local.h
@@ -89,4 +89,11 @@ struct tep_handle {
 	char *trace_clock;
 };
 
+void tep_free_event(struct tep_event *event);
+void tep_free_format_field(struct tep_format_field *field);
+
+unsigned short tep_data2host2(struct tep_handle *pevent, unsigned short data);
+unsigned int tep_data2host4(struct tep_handle *pevent, unsigned int data);
+unsigned long long tep_data2host8(struct tep_handle *pevent, unsigned long long data);
+
 #endif /* _PARSE_EVENTS_INT_H */
diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 73347e2ef81d..4e768357a30d 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -3343,15 +3343,18 @@ tep_find_any_field(struct tep_event *event, const char *name)
 unsigned long long tep_read_number(struct tep_handle *pevent,
 				   const void *ptr, int size)
 {
+	unsigned long long val;
+
 	switch (size) {
 	case 1:
 		return *(unsigned char *)ptr;
 	case 2:
-		return tep_data2host2(pevent, ptr);
+		return tep_data2host2(pevent, *(unsigned short *)ptr);
 	case 4:
-		return tep_data2host4(pevent, ptr);
+		return tep_data2host4(pevent, *(unsigned int *)ptr);
 	case 8:
-		return tep_data2host8(pevent, ptr);
+		memcpy(&val, (ptr), sizeof(unsigned long long));
+		return tep_data2host8(pevent, val);
 	default:
 		/* BUG! */
 		return 0;
@@ -4077,7 +4080,7 @@ static void print_str_arg(struct trace_seq *s, void *data, int size,
 			f = tep_find_any_field(event, arg->string.string);
 			arg->string.offset = f->offset;
 		}
-		str_offset = tep_data2host4(pevent, data + arg->string.offset);
+		str_offset = tep_data2host4(pevent, *(unsigned int *)(data + arg->string.offset));
 		str_offset &= 0xffff;
 		print_str_to_seq(s, format, len_arg, ((char *)data) + str_offset);
 		break;
@@ -4095,7 +4098,7 @@ static void print_str_arg(struct trace_seq *s, void *data, int size,
 			f = tep_find_any_field(event, arg->bitmask.bitmask);
 			arg->bitmask.offset = f->offset;
 		}
-		bitmask_offset = tep_data2host4(pevent, data + arg->bitmask.offset);
+		bitmask_offset = tep_data2host4(pevent, *(unsigned int *)(data + arg->bitmask.offset));
 		bitmask_size = bitmask_offset >> 16;
 		bitmask_offset &= 0xffff;
 		print_bitmask_to_seq(pevent, s, format, len_arg,
diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index 16131e006cfe..ff74dc392268 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -409,22 +409,6 @@ 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);
-unsigned short __tep_data2host2(struct tep_handle *pevent, unsigned short data);
-unsigned int __tep_data2host4(struct tep_handle *pevent, unsigned int data);
-unsigned long long
-__tep_data2host8(struct tep_handle *pevent, unsigned long long data);
-
-#define tep_data2host2(pevent, ptr)	\
-	__tep_data2host2(pevent, *(unsigned short *)(ptr))
-#define tep_data2host4(pevent, ptr)	\
-	__tep_data2host4(pevent, *(unsigned int *)(ptr))
-#define tep_data2host8(pevent, ptr)	\
-({								\
-	unsigned long long __val;				\
-								\
-	memcpy(&__val, (ptr), sizeof(unsigned long long));	\
-	__tep_data2host8(pevent, __val);			\
-})
 
 static inline int tep_host_bigendian(void)
 {
@@ -477,8 +461,6 @@ enum tep_errno tep_parse_format(struct tep_handle *pevent,
 				struct tep_event **eventp,
 				const char *buf,
 				unsigned long size, const char *sys);
-void tep_free_event(struct tep_event *event);
-void tep_free_format_field(struct tep_format_field *field);
 
 void *tep_get_field_raw(struct trace_seq *s, struct tep_event *event,
 			const char *name, struct tep_record *record,
-- 
2.17.2

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

end of thread, other threads:[~2018-11-09 22:38 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-11-06 12:46 [PATCH] tools/lib/traceevent, tools/perf: traceevent API cleanup Tzvetomir Stoyanov
2018-11-06 22:33 ` Steven Rostedt
2018-11-07 11:40   ` Tzvetomir Stoyanov
  -- strict thread matches above, loose matches on Subject: below --
2018-11-09 12:57 Tzvetomir Stoyanov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).