Linux-Trace-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] tools/lib/traceevent, tools/perf: Move struct tep_handler definition in a local header file
@ 2018-10-05 16:22 Steven Rostedt
  2018-10-05 16:28 ` Steven Rostedt
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Steven Rostedt @ 2018-10-05 16:22 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: LKML, Linux Trace Devel, Tzvetomir Stoyanov, Jiri Olsa,
	Ingo Molnar, Namhyung Kim


From: Tzvetomir Stoyanov <tstoyanov@vmware.com>

As traceevent is going to be transferred into a proper library,
its local data should be protected from the library users.
This patch encapsulates struct tep_handler into a local header,
not visible outside of the library. It implements also a bunch
of new APIs, which library users can use to access tep_handler members.

Signed-off-by: Tzvetomir Stoyanov <tstoyanov@vmware.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 tools/lib/traceevent/Build               |   1 +
 tools/lib/traceevent/event-parse-api.c   | 275 +++++++++++++++++++++++++++++++
 tools/lib/traceevent/event-parse-local.h |  92 +++++++++++
 tools/lib/traceevent/event-parse.c       |   2 +
 tools/lib/traceevent/event-parse.h       | 228 ++++---------------------
 tools/lib/traceevent/event-plugin.c      |   1 +
 tools/lib/traceevent/parse-filter.c      |   1 +
 tools/perf/util/trace-event-parse.c      |  25 +--
 tools/perf/util/trace-event-read.c       |   2 +-
 9 files changed, 416 insertions(+), 211 deletions(-)
 create mode 100644 tools/lib/traceevent/event-parse-api.c
 create mode 100644 tools/lib/traceevent/event-parse-local.h

diff --git a/tools/lib/traceevent/Build b/tools/lib/traceevent/Build
index 0050c145d806..ba54bfce0b0b 100644
--- a/tools/lib/traceevent/Build
+++ b/tools/lib/traceevent/Build
@@ -5,6 +5,7 @@ libtraceevent-y += parse-filter.o
 libtraceevent-y += parse-utils.o
 libtraceevent-y += kbuffer-parse.o
 libtraceevent-y += tep_strerror.o
+libtraceevent-y += event-parse-api.o
 
 plugin_jbd2-y         += plugin_jbd2.o
 plugin_hrtimer-y      += plugin_hrtimer.o
diff --git a/tools/lib/traceevent/event-parse-api.c b/tools/lib/traceevent/event-parse-api.c
new file mode 100644
index 000000000000..f4f9dc332ab2
--- /dev/null
+++ b/tools/lib/traceevent/event-parse-api.c
@@ -0,0 +1,275 @@
+// SPDX-License-Identifier: LGPL-2.1
+/*
+ * Copyright (C) 2009, 2010 Red Hat Inc, Steven Rostedt <srostedt@redhat.com>
+ *
+ */
+
+#include "event-parse.h"
+#include "event-parse-local.h"
+#include "event-utils.h"
+
+/**
+ * tep_get_first_event - returns the first event in the events array
+ * @tep: a handle to the tep_handle
+ *
+ * This returns pointer to the first element of the events array
+ * If @tep is NULL, NULL is returned.
+ */
+struct tep_event_format *tep_get_first_event(struct tep_handle *tep)
+{
+	if(tep && tep->events)
+		return tep->events[0];
+
+	return NULL;
+}
+
+/**
+ * tep_get_events_count - get the number of defined events
+ * @tep: a handle to the tep_handle
+ *
+ * This returns number of elements in event array
+ * If @tep is NULL, 0 is returned.
+ */
+int tep_get_events_count(struct tep_handle *tep)
+{
+	if(tep)
+		return tep->nr_events;
+	return 0;
+}
+
+/**
+ * tep_set_flag - set event parser flag
+ * @tep: a handle to the tep_handle
+ * @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
+  */
+void tep_set_flag(struct tep_handle *tep, int flag)
+{
+	if(tep)
+		tep->flags |= flag;
+}
+
+unsigned short __tep_data2host2(struct tep_handle *pevent, unsigned short data)
+{
+	unsigned short swap;
+
+	if (!pevent || pevent->host_bigendian == pevent->file_bigendian)
+		return data;
+
+	swap = ((data & 0xffULL) << 8) |
+		((data & (0xffULL << 8)) >> 8);
+
+	return swap;
+}
+
+unsigned int __tep_data2host4(struct tep_handle *pevent, unsigned int data)
+{
+	unsigned int swap;
+
+	if (!pevent || pevent->host_bigendian == pevent->file_bigendian)
+		return data;
+
+	swap = ((data & 0xffULL) << 24) |
+		((data & (0xffULL << 8)) << 8) |
+		((data & (0xffULL << 16)) >> 8) |
+		((data & (0xffULL << 24)) >> 24);
+
+	return swap;
+}
+
+unsigned long long
+__tep_data2host8(struct tep_handle *pevent, unsigned long long data)
+{
+	unsigned long long swap;
+
+	if (!pevent || pevent->host_bigendian == pevent->file_bigendian)
+		return data;
+
+	swap = ((data & 0xffULL) << 56) |
+		((data & (0xffULL << 8)) << 40) |
+		((data & (0xffULL << 16)) << 24) |
+		((data & (0xffULL << 24)) << 8) |
+		((data & (0xffULL << 32)) >> 8) |
+		((data & (0xffULL << 40)) >> 24) |
+		((data & (0xffULL << 48)) >> 40) |
+		((data & (0xffULL << 56)) >> 56);
+
+	return swap;
+}
+
+/**
+ * tep_get_header_page_size - get size of the header page
+ * @pevent: a handle to the tep_handle
+ *
+ * This returns size of the header page
+ * If @pevent is NULL, 0 is returned.
+ */
+int tep_get_header_page_size(struct tep_handle *pevent)
+{
+	if(pevent)
+		return pevent->header_page_size_size;
+	return 0;
+}
+
+/**
+ * tep_get_cpus - get the number of CPUs
+ * @pevent: a handle to the tep_handle
+ *
+ * This returns the number of CPUs
+ * If @pevent is NULL, 0 is returned.
+ */
+int tep_get_cpus(struct tep_handle *pevent)
+{
+	if(pevent)
+		return pevent->cpus;
+	return 0;
+}
+
+/**
+ * tep_set_cpus - set the number of CPUs
+ * @pevent: a handle to the tep_handle
+ *
+ * This sets the number of CPUs
+ */
+void tep_set_cpus(struct tep_handle *pevent, int cpus)
+{
+	if(pevent)
+		pevent->cpus = cpus;
+}
+
+/**
+ * tep_get_long_size - get the size of a long integer on the current machine
+ * @pevent: a handle to the tep_handle
+ *
+ * This returns the size of a long integer on the current machine
+ * If @pevent is NULL, 0 is returned.
+ */
+int tep_get_long_size(struct tep_handle *pevent)
+{
+	if(pevent)
+		return pevent->long_size;
+	return 0;
+}
+
+/**
+ * tep_set_long_size - set the size of a long integer on the current 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
+ */
+void tep_set_long_size(struct tep_handle *pevent, int long_size)
+{
+	if(pevent)
+		pevent->long_size = long_size;
+}
+
+/**
+ * tep_get_page_size - get the size of a memory page on the current machine
+ * @pevent: a handle to the tep_handle
+ *
+ * This returns the size of a memory page on the current machine
+ * If @pevent is NULL, 0 is returned.
+ */
+int tep_get_page_size(struct tep_handle *pevent)
+{
+	if(pevent)
+		return pevent->page_size;
+	return 0;
+}
+
+/**
+ * tep_set_page_size - set the size of a memory page on the current 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
+ */
+void tep_set_page_size(struct tep_handle *pevent, int _page_size)
+{
+	if(pevent)
+		pevent->page_size = _page_size;
+}
+
+/**
+ * tep_is_file_bigendian - get if the file is in big endian order
+ * @pevent: a handle to the tep_handle
+ *
+ * This returns if the file is in big endian order
+ * If @pevent is NULL, 0 is returned.
+ */
+int tep_is_file_bigendian(struct tep_handle *pevent)
+{
+	if(pevent)
+		return pevent->file_bigendian;
+	return 0;
+}
+
+/**
+ * tep_set_file_bigendian - set if the file is in big endian order
+ * @pevent: a handle to the tep_handle
+ * @endian: non zero, if the file is in big endian order
+ *
+ * This sets if the file is in big endian order
+ */
+void tep_set_file_bigendian(struct tep_handle *pevent, enum tep_endian endian)
+{
+	if(pevent)
+		pevent->file_bigendian = endian;
+}
+
+/**
+ * tep_is_host_bigendian - get if the order of the current host is big endian
+ * @pevent: a handle to the tep_handle
+ *
+ * This gets if the order of the current host is big endian
+ * If @pevent is NULL, 0 is returned.
+ */
+int tep_is_host_bigendian(struct tep_handle *pevent)
+{
+	if(pevent)
+		return pevent->host_bigendian;
+	return 0;
+}
+
+/**
+ * tep_set_host_bigendian - set the order of the local host
+ * @pevent: a handle to the tep_handle
+ * @endian: non zero, if the local host has big endian order
+ *
+ * This sets the order of the local host
+ */
+void tep_set_host_bigendian(struct tep_handle *pevent, enum tep_endian endian)
+{
+	if(pevent)
+		pevent->host_bigendian = endian;
+}
+
+/**
+ * tep_is_latency_format - get if the latency output format is configured
+ * @pevent: a handle to the tep_handle
+ *
+ * This gets if the latency output format is configured
+ * If @pevent is NULL, 0 is returned.
+ */
+int tep_is_latency_format(struct tep_handle *pevent)
+{
+	if(pevent)
+		return pevent->latency_format;
+	return 0;
+}
+
+/**
+ * tep_set_latency_format - set the latency output format
+ * @pevent: a handle to the tep_handle
+ * @lat: non zero for latency output format
+ *
+ * This sets the latency output format
+  */
+void tep_set_latency_format(struct tep_handle *pevent, int lat)
+{
+	if(pevent)
+		pevent->latency_format = lat;
+}
diff --git a/tools/lib/traceevent/event-parse-local.h b/tools/lib/traceevent/event-parse-local.h
new file mode 100644
index 000000000000..b9bddde577f8
--- /dev/null
+++ b/tools/lib/traceevent/event-parse-local.h
@@ -0,0 +1,92 @@
+// SPDX-License-Identifier: LGPL-2.1
+/*
+ * Copyright (C) 2009, 2010 Red Hat Inc, Steven Rostedt <srostedt@redhat.com>
+ *
+ */
+
+#ifndef _PARSE_EVENTS_INT_H
+#define _PARSE_EVENTS_INT_H
+
+struct cmdline;
+struct cmdline_list;
+struct func_map;
+struct func_list;
+struct event_handler;
+struct func_resolver;
+
+struct tep_handle {
+	int ref_count;
+
+	int header_page_ts_offset;
+	int header_page_ts_size;
+	int header_page_size_offset;
+	int header_page_size_size;
+	int header_page_data_offset;
+	int header_page_data_size;
+	int header_page_overwrite;
+
+	enum tep_endian file_bigendian;
+	enum tep_endian host_bigendian;
+
+	int latency_format;
+
+	int old_format;
+
+	int cpus;
+	int long_size;
+	int page_size;
+
+	struct cmdline *cmdlines;
+	struct cmdline_list *cmdlist;
+	int cmdline_count;
+
+	struct func_map *func_map;
+	struct func_resolver *func_resolver;
+	struct func_list *funclist;
+	unsigned int func_count;
+
+	struct printk_map *printk_map;
+	struct printk_list *printklist;
+	unsigned int printk_count;
+
+
+	struct tep_event_format **events;
+	int nr_events;
+	struct tep_event_format **sort_events;
+	enum tep_event_sort_type last_type;
+
+	int type_offset;
+	int type_size;
+
+	int pid_offset;
+	int pid_size;
+
+	int pc_offset;
+	int pc_size;
+
+	int flags_offset;
+	int flags_size;
+
+	int ld_offset;
+	int ld_size;
+
+	int print_raw;
+
+	int test_filters;
+
+	int flags;
+
+	struct tep_format_field *bprint_ip_field;
+	struct tep_format_field *bprint_fmt_field;
+	struct tep_format_field *bprint_buf_field;
+
+	struct event_handler *handlers;
+	struct tep_function_handler *func_handlers;
+
+	/* cache */
+	struct tep_event_format *last_event;
+
+	char *trace_clock;
+};
+
+#endif /* _PARSE_EVENTS_INT_H */
diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 233179a712d6..3692f29fee46 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -22,6 +22,8 @@
 
 #include <netinet/in.h>
 #include "event-parse.h"
+
+#include "event-parse-local.h"
 #include "event-utils.h"
 #include "trace-seq.h"
 
diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index 9c29a5f7aa39..16bf4c890b6f 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -405,149 +405,18 @@ void tep_print_plugins(struct trace_seq *s,
 			const char *prefix, const char *suffix,
 			const struct tep_plugin_list *list);
 
-struct cmdline;
-struct cmdline_list;
-struct func_map;
-struct func_list;
-struct event_handler;
-struct func_resolver;
-
+/* tep_handle */
 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);
 
-struct tep_handle {
-	int ref_count;
-
-	int header_page_ts_offset;
-	int header_page_ts_size;
-	int header_page_size_offset;
-	int header_page_size_size;
-	int header_page_data_offset;
-	int header_page_data_size;
-	int header_page_overwrite;
-
-	int file_bigendian;
-	int host_bigendian;
-
-	int latency_format;
-
-	int old_format;
-
-	int cpus;
-	int long_size;
-	int page_size;
-
-	struct cmdline *cmdlines;
-	struct cmdline_list *cmdlist;
-	int cmdline_count;
-
-	struct func_map *func_map;
-	struct func_resolver *func_resolver;
-	struct func_list *funclist;
-	unsigned int func_count;
-
-	struct printk_map *printk_map;
-	struct printk_list *printklist;
-	unsigned int printk_count;
-
-
-	struct tep_event_format **events;
-	int nr_events;
-	struct tep_event_format **sort_events;
-	enum tep_event_sort_type last_type;
-
-	int type_offset;
-	int type_size;
-
-	int pid_offset;
-	int pid_size;
-
- 	int pc_offset;
-	int pc_size;
-
-	int flags_offset;
-	int flags_size;
-
-	int ld_offset;
-	int ld_size;
-
-	int print_raw;
-
-	int test_filters;
-
-	int flags;
-
-	struct tep_format_field *bprint_ip_field;
-	struct tep_format_field *bprint_fmt_field;
-	struct tep_format_field *bprint_buf_field;
-
-	struct event_handler *handlers;
-	struct tep_function_handler *func_handlers;
-
-	/* cache */
-	struct tep_event_format *last_event;
-
-	char *trace_clock;
-};
-
-static inline void tep_set_flag(struct tep_handle *pevent, int flag)
-{
-	pevent->flags |= flag;
-}
-
-static inline unsigned short
-__tep_data2host2(struct tep_handle *pevent, unsigned short data)
-{
-	unsigned short swap;
-
-	if (pevent->host_bigendian == pevent->file_bigendian)
-		return data;
-
-	swap = ((data & 0xffULL) << 8) |
-		((data & (0xffULL << 8)) >> 8);
-
-	return swap;
-}
-
-static inline unsigned int
-__tep_data2host4(struct tep_handle *pevent, unsigned int data)
-{
-	unsigned int swap;
-
-	if (pevent->host_bigendian == pevent->file_bigendian)
-		return data;
-
-	swap = ((data & 0xffULL) << 24) |
-		((data & (0xffULL << 8)) << 8) |
-		((data & (0xffULL << 16)) >> 8) |
-		((data & (0xffULL << 24)) >> 24);
-
-	return swap;
-}
-
-static inline unsigned long long
-__tep_data2host8(struct tep_handle *pevent, unsigned long long data)
-{
-	unsigned long long swap;
-
-	if (pevent->host_bigendian == pevent->file_bigendian)
-		return data;
-
-	swap = ((data & 0xffULL) << 56) |
-		((data & (0xffULL << 8)) << 40) |
-		((data & (0xffULL << 16)) << 24) |
-		((data & (0xffULL << 24)) << 8) |
-		((data & (0xffULL << 32)) >> 8) |
-		((data & (0xffULL << 40)) >> 24) |
-		((data & (0xffULL << 48)) >> 40) |
-		((data & (0xffULL << 56)) >> 56);
-
-	return swap;
-}
-
-#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)					\
+#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;				\
 								\
@@ -655,11 +524,12 @@ unsigned long long tep_read_number(struct tep_handle *pevent, const void *ptr, i
 int tep_read_number_field(struct tep_format_field *field, const void *data,
 			  unsigned long long *value);
 
+struct tep_event_format *tep_get_first_event(struct tep_handle *tep);
+int tep_get_events_count(struct tep_handle *tep);
 struct tep_event_format *tep_find_event(struct tep_handle *pevent, int id);
 
 struct tep_event_format *
 tep_find_event_by_name(struct tep_handle *pevent, const char *sys, const char *name);
-
 struct tep_event_format *
 tep_find_event_by_record(struct tep_handle *pevent, struct tep_record *record);
 
@@ -689,65 +559,23 @@ struct tep_event_format **tep_list_events(struct tep_handle *pevent, enum tep_ev
 struct tep_format_field **tep_event_common_fields(struct tep_event_format *event);
 struct tep_format_field **tep_event_fields(struct tep_event_format *event);
 
-static inline int tep_get_cpus(struct tep_handle *pevent)
-{
-	return pevent->cpus;
-}
-
-static inline void tep_set_cpus(struct tep_handle *pevent, int cpus)
-{
-	pevent->cpus = cpus;
-}
-
-static inline int tep_get_long_size(struct tep_handle *pevent)
-{
-	return pevent->long_size;
-}
-
-static inline void tep_set_long_size(struct tep_handle *pevent, int long_size)
-{
-	pevent->long_size = long_size;
-}
-
-static inline int tep_get_page_size(struct tep_handle *pevent)
-{
-	return pevent->page_size;
-}
-
-static inline void tep_set_page_size(struct tep_handle *pevent, int _page_size)
-{
-	pevent->page_size = _page_size;
-}
-
-static inline int tep_is_file_bigendian(struct tep_handle *pevent)
-{
-	return pevent->file_bigendian;
-}
-
-static inline void tep_set_file_bigendian(struct tep_handle *pevent, int endian)
-{
-	pevent->file_bigendian = endian;
-}
-
-static inline int tep_is_host_bigendian(struct tep_handle *pevent)
-{
-	return pevent->host_bigendian;
-}
-
-static inline void tep_set_host_bigendian(struct tep_handle *pevent, int endian)
-{
-	pevent->host_bigendian = endian;
-}
-
-static inline int tep_is_latency_format(struct tep_handle *pevent)
-{
-	return pevent->latency_format;
-}
-
-static inline void tep_set_latency_format(struct tep_handle *pevent, int lat)
-{
-	pevent->latency_format = lat;
-}
+enum tep_endian {
+        TEP_LITTLE_ENDIAN = 0,
+        TEP_BIG_ENDIAN
+};
+int tep_get_cpus(struct tep_handle *pevent);
+void tep_set_cpus(struct tep_handle *pevent, int cpus);
+int tep_get_long_size(struct tep_handle *pevent);
+void tep_set_long_size(struct tep_handle *pevent, int long_size);
+int tep_get_page_size(struct tep_handle *pevent);
+void tep_set_page_size(struct tep_handle *pevent, int _page_size);
+int tep_is_file_bigendian(struct tep_handle *pevent);
+void tep_set_file_bigendian(struct tep_handle *pevent, enum tep_endian endian);
+int tep_is_host_bigendian(struct tep_handle *pevent);
+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);
 
 struct tep_handle *tep_alloc(void);
 void tep_free(struct tep_handle *pevent);
diff --git a/tools/lib/traceevent/event-plugin.c b/tools/lib/traceevent/event-plugin.c
index 46eb64eb0c2e..e74f16c88398 100644
--- a/tools/lib/traceevent/event-plugin.c
+++ b/tools/lib/traceevent/event-plugin.c
@@ -14,6 +14,7 @@
 #include <unistd.h>
 #include <dirent.h>
 #include "event-parse.h"
+#include "event-parse-local.h"
 #include "event-utils.h"
 #include "trace-seq.h"
 
diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c
index d64b6128fa7d..ed87cb56713d 100644
--- a/tools/lib/traceevent/parse-filter.c
+++ b/tools/lib/traceevent/parse-filter.c
@@ -11,6 +11,7 @@
 #include <sys/types.h>
 
 #include "event-parse.h"
+#include "event-parse-local.h"
 #include "event-utils.h"
 
 #define COMM "COMM"
diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
index a4d7de1c96d1..9b78160e23a2 100644
--- a/tools/perf/util/trace-event-parse.c
+++ b/tools/perf/util/trace-event-parse.c
@@ -37,10 +37,11 @@ static int get_common_field(struct scripting_context *context,
 	struct tep_format_field *field;
 
 	if (!*size) {
-		if (!pevent->events)
+
+		event = tep_get_first_event(pevent);
+		if (!event)
 			return 0;
 
-		event = pevent->events[0];
 		field = tep_find_common_field(event, type);
 		if (!field)
 			return 0;
@@ -192,25 +193,29 @@ struct tep_event_format *trace_find_next_event(struct tep_handle *pevent,
 					       struct tep_event_format *event)
 {
 	static int idx;
+	int events_count;
+	struct tep_event_format *all_events;
 
-	if (!pevent || !pevent->events)
+	all_events = tep_get_first_event(pevent);
+	events_count = tep_get_events_count(pevent);
+	if (!pevent || !all_events || events_count < 1)
 		return NULL;
 
 	if (!event) {
 		idx = 0;
-		return pevent->events[0];
+		return all_events;
 	}
 
-	if (idx < pevent->nr_events && event == pevent->events[idx]) {
+	if (idx < events_count && event == (all_events + idx)) {
 		idx++;
-		if (idx == pevent->nr_events)
+		if (idx == events_count)
 			return NULL;
-		return pevent->events[idx];
+		return (all_events + idx);
 	}
 
-	for (idx = 1; idx < pevent->nr_events; idx++) {
-		if (event == pevent->events[idx - 1])
-			return pevent->events[idx];
+	for (idx = 1; idx < events_count; idx++) {
+		if (event == (all_events + (idx - 1)))
+			return (all_events + idx);
 	}
 	return NULL;
 }
diff --git a/tools/perf/util/trace-event-read.c b/tools/perf/util/trace-event-read.c
index b98ee2a2eb44..cdf6de82a507 100644
--- a/tools/perf/util/trace-event-read.c
+++ b/tools/perf/util/trace-event-read.c
@@ -241,7 +241,7 @@ static int read_header_files(struct tep_handle *pevent)
 		 * The commit field in the page is of type long,
 		 * use that instead, since it represents the kernel.
 		 */
-		tep_set_long_size(pevent, pevent->header_page_size_size);
+		tep_set_long_size(pevent, tep_get_header_page_size(pevent));
 	}
 	free(header_page);
 
-- 
2.13.6

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

* Re: [PATCH] tools/lib/traceevent, tools/perf: Move struct tep_handler definition in a local header file
  2018-10-05 16:22 [PATCH] tools/lib/traceevent, tools/perf: Move struct tep_handler definition in a local header file Steven Rostedt
@ 2018-10-05 16:28 ` Steven Rostedt
  2018-10-08 17:32   ` Arnaldo Carvalho de Melo
  2018-10-08 17:31 ` Arnaldo Carvalho de Melo
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: Steven Rostedt @ 2018-10-05 16:28 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: LKML, Linux Trace Devel, Tzvetomir Stoyanov, Jiri Olsa,
	Ingo Molnar, Namhyung Kim

On Fri, 5 Oct 2018 12:22:25 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> +/**
> + * tep_get_first_event - returns the first event in the events array
> + * @tep: a handle to the tep_handle
> + *
> + * This returns pointer to the first element of the events array
> + * If @tep is NULL, NULL is returned.
> + */
> +struct tep_event_format *tep_get_first_event(struct tep_handle *tep)
> +{
> +	if(tep && tep->events)
> +		return tep->events[0];
> +
> +	return NULL;
> +}
> +

Hold off on pulling this in. This patch has some whitespace issues that
need to be cleaned up.

Thanks!

-- Steve

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

* Re: [PATCH] tools/lib/traceevent, tools/perf: Move struct tep_handler definition in a local header file
  2018-10-05 16:22 [PATCH] tools/lib/traceevent, tools/perf: Move struct tep_handler definition in a local header file Steven Rostedt
  2018-10-05 16:28 ` Steven Rostedt
@ 2018-10-08 17:31 ` Arnaldo Carvalho de Melo
  2018-10-09  5:33 ` [tip:perf/core] tools lib traceevent, perf tools: " tip-bot for Tzvetomir Stoyanov
  2019-07-26  3:58 ` [PATCH] tools/lib/traceevent, tools/perf: " Andres Freund
  3 siblings, 0 replies; 14+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-10-08 17:31 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: LKML, Linux Trace Devel, Tzvetomir Stoyanov, Jiri Olsa,
	Ingo Molnar, Namhyung Kim

Em Fri, Oct 05, 2018 at 12:22:25PM -0400, Steven Rostedt escreveu:
> 
> From: Tzvetomir Stoyanov <tstoyanov@vmware.com>
> 
> As traceevent is going to be transferred into a proper library,
> its local data should be protected from the library users.
> This patch encapsulates struct tep_handler into a local header,
> not visible outside of the library. It implements also a bunch
> of new APIs, which library users can use to access tep_handler members.
> 
> Signed-off-by: Tzvetomir Stoyanov <tstoyanov@vmware.com>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

Thanks, applied to perf/core.

- Arnaldo

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

* Re: [PATCH] tools/lib/traceevent, tools/perf: Move struct tep_handler definition in a local header file
  2018-10-05 16:28 ` Steven Rostedt
@ 2018-10-08 17:32   ` Arnaldo Carvalho de Melo
  2018-10-08 19:54     ` Steven Rostedt
  0 siblings, 1 reply; 14+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-10-08 17:32 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: LKML, Linux Trace Devel, Tzvetomir Stoyanov, Jiri Olsa,
	Ingo Molnar, Namhyung Kim

Em Fri, Oct 05, 2018 at 12:28:46PM -0400, Steven Rostedt escreveu:
> On Fri, 5 Oct 2018 12:22:25 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > +/**
> > + * tep_get_first_event - returns the first event in the events array
> > + * @tep: a handle to the tep_handle
> > + *
> > + * This returns pointer to the first element of the events array
> > + * If @tep is NULL, NULL is returned.
> > + */
> > +struct tep_event_format *tep_get_first_event(struct tep_handle *tep)
> > +{
> > +	if(tep && tep->events)
> > +		return tep->events[0];
> > +
> > +	return NULL;
> > +}
> > +
> 
> Hold off on pulling this in. This patch has some whitespace issues that
> need to be cleaned up.

Ah, that if( thing? ?I can fix it here, holler if there is more than
that.

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

* Re: [PATCH] tools/lib/traceevent, tools/perf: Move struct tep_handler definition in a local header file
  2018-10-08 17:32   ` Arnaldo Carvalho de Melo
@ 2018-10-08 19:54     ` Steven Rostedt
  0 siblings, 0 replies; 14+ messages in thread
From: Steven Rostedt @ 2018-10-08 19:54 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: LKML, Linux Trace Devel, Tzvetomir Stoyanov, Jiri Olsa,
	Ingo Molnar, Namhyung Kim

On Mon, 8 Oct 2018 14:32:24 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Fri, Oct 05, 2018 at 12:28:46PM -0400, Steven Rostedt escreveu:
> > On Fri, 5 Oct 2018 12:22:25 -0400
> > Steven Rostedt <rostedt@goodmis.org> wrote:
> >   
> > > +/**
> > > + * tep_get_first_event - returns the first event in the events array
> > > + * @tep: a handle to the tep_handle
> > > + *
> > > + * This returns pointer to the first element of the events array
> > > + * If @tep is NULL, NULL is returned.
> > > + */
> > > +struct tep_event_format *tep_get_first_event(struct tep_handle *tep)
> > > +{
> > > +	if(tep && tep->events)
> > > +		return tep->events[0];
> > > +
> > > +	return NULL;
> > > +}
> > > +  
> > 
> > Hold off on pulling this in. This patch has some whitespace issues that
> > need to be cleaned up.  
> 
> Ah, that if( thing? ?I can fix it here, holler if there is more than
> that.

Well, here's a fixed up version anyway...

-- Steve


>From 96dd7937a31d3a715c450c9a67f62a0022cd6a7c Mon Sep 17 00:00:00 2001
From: Tzvetomir Stoyanov <tstoyanov@vmware.com>
Date: Mon, 8 Oct 2018 15:55:00 +0300
Subject: [PATCH] tools/lib/traceevent, tools/perf: Move struct tep_handler
 definition in a local header file

As traceevent is going to be transferred into a proper library,
its local data should be protected from the library users.
This patch capsulates struct tep_handler into a local header,
not visible outside of the library. It implements also a bunch
of new APIs, which library users can use to access tep_handler members.
Version 3 of the patch - fixed whitespace issues.

Signed-off-by: Tzvetomir Stoyanov <tstoyanov@vmware.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 tools/lib/traceevent/Build               |   1 +
 tools/lib/traceevent/event-parse-api.c   | 275 +++++++++++++++++++++++++++++++
 tools/lib/traceevent/event-parse-local.h |  92 +++++++++++
 tools/lib/traceevent/event-parse.c       |   2 +
 tools/lib/traceevent/event-parse.h       | 230 ++++----------------------
 tools/lib/traceevent/event-plugin.c      |   1 +
 tools/lib/traceevent/parse-filter.c      |   1 +
 tools/perf/util/trace-event-parse.c      |  25 +--
 tools/perf/util/trace-event-read.c       |   2 +-
 9 files changed, 417 insertions(+), 212 deletions(-)
 create mode 100644 tools/lib/traceevent/event-parse-api.c
 create mode 100644 tools/lib/traceevent/event-parse-local.h

diff --git a/tools/lib/traceevent/Build b/tools/lib/traceevent/Build
index 0050c145d806..ba54bfce0b0b 100644
--- a/tools/lib/traceevent/Build
+++ b/tools/lib/traceevent/Build
@@ -5,6 +5,7 @@ libtraceevent-y += parse-filter.o
 libtraceevent-y += parse-utils.o
 libtraceevent-y += kbuffer-parse.o
 libtraceevent-y += tep_strerror.o
+libtraceevent-y += event-parse-api.o
 
 plugin_jbd2-y         += plugin_jbd2.o
 plugin_hrtimer-y      += plugin_hrtimer.o
diff --git a/tools/lib/traceevent/event-parse-api.c b/tools/lib/traceevent/event-parse-api.c
new file mode 100644
index 000000000000..3b9371fb41cf
--- /dev/null
+++ b/tools/lib/traceevent/event-parse-api.c
@@ -0,0 +1,275 @@
+// SPDX-License-Identifier: LGPL-2.1
+/*
+ * Copyright (C) 2009, 2010 Red Hat Inc, Steven Rostedt <srostedt@redhat.com>
+ *
+ */
+
+#include "event-parse.h"
+#include "event-parse-local.h"
+#include "event-utils.h"
+
+/**
+ * tep_get_first_event - returns the first event in the events array
+ * @tep: a handle to the tep_handle
+ *
+ * This returns pointer to the first element of the events array
+ * If @tep is NULL, NULL is returned.
+ */
+struct tep_event_format *tep_get_first_event(struct tep_handle *tep)
+{
+	if (tep && tep->events)
+		return tep->events[0];
+
+	return NULL;
+}
+
+/**
+ * tep_get_events_count - get the number of defined events
+ * @tep: a handle to the tep_handle
+ *
+ * This returns number of elements in event array
+ * If @tep is NULL, 0 is returned.
+ */
+int tep_get_events_count(struct tep_handle *tep)
+{
+	if (tep)
+		return tep->nr_events;
+	return 0;
+}
+
+/**
+ * tep_set_flag - set event parser flag
+ * @tep: a handle to the tep_handle
+ * @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
+ */
+void tep_set_flag(struct tep_handle *tep, int flag)
+{
+	if (tep)
+		tep->flags |= flag;
+}
+
+unsigned short __tep_data2host2(struct tep_handle *pevent, unsigned short data)
+{
+	unsigned short swap;
+
+	if (!pevent || pevent->host_bigendian == pevent->file_bigendian)
+		return data;
+
+	swap = ((data & 0xffULL) << 8) |
+		((data & (0xffULL << 8)) >> 8);
+
+	return swap;
+}
+
+unsigned int __tep_data2host4(struct tep_handle *pevent, unsigned int data)
+{
+	unsigned int swap;
+
+	if (!pevent || pevent->host_bigendian == pevent->file_bigendian)
+		return data;
+
+	swap = ((data & 0xffULL) << 24) |
+		((data & (0xffULL << 8)) << 8) |
+		((data & (0xffULL << 16)) >> 8) |
+		((data & (0xffULL << 24)) >> 24);
+
+	return swap;
+}
+
+unsigned long long
+__tep_data2host8(struct tep_handle *pevent, unsigned long long data)
+{
+	unsigned long long swap;
+
+	if (!pevent || pevent->host_bigendian == pevent->file_bigendian)
+		return data;
+
+	swap = ((data & 0xffULL) << 56) |
+		((data & (0xffULL << 8)) << 40) |
+		((data & (0xffULL << 16)) << 24) |
+		((data & (0xffULL << 24)) << 8) |
+		((data & (0xffULL << 32)) >> 8) |
+		((data & (0xffULL << 40)) >> 24) |
+		((data & (0xffULL << 48)) >> 40) |
+		((data & (0xffULL << 56)) >> 56);
+
+	return swap;
+}
+
+/**
+ * tep_get_header_page_size - get size of the header page
+ * @pevent: a handle to the tep_handle
+ *
+ * This returns size of the header page
+ * If @pevent is NULL, 0 is returned.
+ */
+int tep_get_header_page_size(struct tep_handle *pevent)
+{
+	if (pevent)
+		return pevent->header_page_size_size;
+	return 0;
+}
+
+/**
+ * tep_get_cpus - get the number of CPUs
+ * @pevent: a handle to the tep_handle
+ *
+ * This returns the number of CPUs
+ * If @pevent is NULL, 0 is returned.
+ */
+int tep_get_cpus(struct tep_handle *pevent)
+{
+	if (pevent)
+		return pevent->cpus;
+	return 0;
+}
+
+/**
+ * tep_set_cpus - set the number of CPUs
+ * @pevent: a handle to the tep_handle
+ *
+ * This sets the number of CPUs
+ */
+void tep_set_cpus(struct tep_handle *pevent, int cpus)
+{
+	if (pevent)
+		pevent->cpus = cpus;
+}
+
+/**
+ * tep_get_long_size - get the size of a long integer on the current machine
+ * @pevent: a handle to the tep_handle
+ *
+ * This returns the size of a long integer on the current machine
+ * If @pevent is NULL, 0 is returned.
+ */
+int tep_get_long_size(struct tep_handle *pevent)
+{
+	if (pevent)
+		return pevent->long_size;
+	return 0;
+}
+
+/**
+ * tep_set_long_size - set the size of a long integer on the current 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
+ */
+void tep_set_long_size(struct tep_handle *pevent, int long_size)
+{
+	if (pevent)
+		pevent->long_size = long_size;
+}
+
+/**
+ * tep_get_page_size - get the size of a memory page on the current machine
+ * @pevent: a handle to the tep_handle
+ *
+ * This returns the size of a memory page on the current machine
+ * If @pevent is NULL, 0 is returned.
+ */
+int tep_get_page_size(struct tep_handle *pevent)
+{
+	if (pevent)
+		return pevent->page_size;
+	return 0;
+}
+
+/**
+ * tep_set_page_size - set the size of a memory page on the current 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
+ */
+void tep_set_page_size(struct tep_handle *pevent, int _page_size)
+{
+	if (pevent)
+		pevent->page_size = _page_size;
+}
+
+/**
+ * tep_is_file_bigendian - get if the file is in big endian order
+ * @pevent: a handle to the tep_handle
+ *
+ * This returns if the file is in big endian order
+ * If @pevent is NULL, 0 is returned.
+ */
+int tep_is_file_bigendian(struct tep_handle *pevent)
+{
+	if (pevent)
+		return pevent->file_bigendian;
+	return 0;
+}
+
+/**
+ * tep_set_file_bigendian - set if the file is in big endian order
+ * @pevent: a handle to the tep_handle
+ * @endian: non zero, if the file is in big endian order
+ *
+ * This sets if the file is in big endian order
+ */
+void tep_set_file_bigendian(struct tep_handle *pevent, enum tep_endian endian)
+{
+	if (pevent)
+		pevent->file_bigendian = endian;
+}
+
+/**
+ * tep_is_host_bigendian - get if the order of the current host is big endian
+ * @pevent: a handle to the tep_handle
+ *
+ * This gets if the order of the current host is big endian
+ * If @pevent is NULL, 0 is returned.
+ */
+int tep_is_host_bigendian(struct tep_handle *pevent)
+{
+	if (pevent)
+		return pevent->host_bigendian;
+	return 0;
+}
+
+/**
+ * tep_set_host_bigendian - set the order of the local host
+ * @pevent: a handle to the tep_handle
+ * @endian: non zero, if the local host has big endian order
+ *
+ * This sets the order of the local host
+ */
+void tep_set_host_bigendian(struct tep_handle *pevent, enum tep_endian endian)
+{
+	if (pevent)
+		pevent->host_bigendian = endian;
+}
+
+/**
+ * tep_is_latency_format - get if the latency output format is configured
+ * @pevent: a handle to the tep_handle
+ *
+ * This gets if the latency output format is configured
+ * If @pevent is NULL, 0 is returned.
+ */
+int tep_is_latency_format(struct tep_handle *pevent)
+{
+	if (pevent)
+		return pevent->latency_format;
+	return 0;
+}
+
+/**
+ * tep_set_latency_format - set the latency output format
+ * @pevent: a handle to the tep_handle
+ * @lat: non zero for latency output format
+ *
+ * This sets the latency output format
+ */
+void tep_set_latency_format(struct tep_handle *pevent, int lat)
+{
+	if (pevent)
+		pevent->latency_format = lat;
+}
diff --git a/tools/lib/traceevent/event-parse-local.h b/tools/lib/traceevent/event-parse-local.h
new file mode 100644
index 000000000000..b9bddde577f8
--- /dev/null
+++ b/tools/lib/traceevent/event-parse-local.h
@@ -0,0 +1,92 @@
+// SPDX-License-Identifier: LGPL-2.1
+/*
+ * Copyright (C) 2009, 2010 Red Hat Inc, Steven Rostedt <srostedt@redhat.com>
+ *
+ */
+
+#ifndef _PARSE_EVENTS_INT_H
+#define _PARSE_EVENTS_INT_H
+
+struct cmdline;
+struct cmdline_list;
+struct func_map;
+struct func_list;
+struct event_handler;
+struct func_resolver;
+
+struct tep_handle {
+	int ref_count;
+
+	int header_page_ts_offset;
+	int header_page_ts_size;
+	int header_page_size_offset;
+	int header_page_size_size;
+	int header_page_data_offset;
+	int header_page_data_size;
+	int header_page_overwrite;
+
+	enum tep_endian file_bigendian;
+	enum tep_endian host_bigendian;
+
+	int latency_format;
+
+	int old_format;
+
+	int cpus;
+	int long_size;
+	int page_size;
+
+	struct cmdline *cmdlines;
+	struct cmdline_list *cmdlist;
+	int cmdline_count;
+
+	struct func_map *func_map;
+	struct func_resolver *func_resolver;
+	struct func_list *funclist;
+	unsigned int func_count;
+
+	struct printk_map *printk_map;
+	struct printk_list *printklist;
+	unsigned int printk_count;
+
+
+	struct tep_event_format **events;
+	int nr_events;
+	struct tep_event_format **sort_events;
+	enum tep_event_sort_type last_type;
+
+	int type_offset;
+	int type_size;
+
+	int pid_offset;
+	int pid_size;
+
+	int pc_offset;
+	int pc_size;
+
+	int flags_offset;
+	int flags_size;
+
+	int ld_offset;
+	int ld_size;
+
+	int print_raw;
+
+	int test_filters;
+
+	int flags;
+
+	struct tep_format_field *bprint_ip_field;
+	struct tep_format_field *bprint_fmt_field;
+	struct tep_format_field *bprint_buf_field;
+
+	struct event_handler *handlers;
+	struct tep_function_handler *func_handlers;
+
+	/* cache */
+	struct tep_event_format *last_event;
+
+	char *trace_clock;
+};
+
+#endif /* _PARSE_EVENTS_INT_H */
diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 233179a712d6..3692f29fee46 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -22,6 +22,8 @@
 
 #include <netinet/in.h>
 #include "event-parse.h"
+
+#include "event-parse-local.h"
 #include "event-utils.h"
 #include "trace-seq.h"
 
diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index 9c29a5f7aa39..856334677c59 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -405,154 +405,23 @@ void tep_print_plugins(struct trace_seq *s,
 			const char *prefix, const char *suffix,
 			const struct tep_plugin_list *list);
 
-struct cmdline;
-struct cmdline_list;
-struct func_map;
-struct func_list;
-struct event_handler;
-struct func_resolver;
-
+/* tep_handle */
 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);
 
-struct tep_handle {
-	int ref_count;
-
-	int header_page_ts_offset;
-	int header_page_ts_size;
-	int header_page_size_offset;
-	int header_page_size_size;
-	int header_page_data_offset;
-	int header_page_data_size;
-	int header_page_overwrite;
-
-	int file_bigendian;
-	int host_bigendian;
-
-	int latency_format;
-
-	int old_format;
-
-	int cpus;
-	int long_size;
-	int page_size;
-
-	struct cmdline *cmdlines;
-	struct cmdline_list *cmdlist;
-	int cmdline_count;
-
-	struct func_map *func_map;
-	struct func_resolver *func_resolver;
-	struct func_list *funclist;
-	unsigned int func_count;
-
-	struct printk_map *printk_map;
-	struct printk_list *printklist;
-	unsigned int printk_count;
-
-
-	struct tep_event_format **events;
-	int nr_events;
-	struct tep_event_format **sort_events;
-	enum tep_event_sort_type last_type;
-
-	int type_offset;
-	int type_size;
-
-	int pid_offset;
-	int pid_size;
-
- 	int pc_offset;
-	int pc_size;
-
-	int flags_offset;
-	int flags_size;
-
-	int ld_offset;
-	int ld_size;
-
-	int print_raw;
-
-	int test_filters;
-
-	int flags;
-
-	struct tep_format_field *bprint_ip_field;
-	struct tep_format_field *bprint_fmt_field;
-	struct tep_format_field *bprint_buf_field;
-
-	struct event_handler *handlers;
-	struct tep_function_handler *func_handlers;
-
-	/* cache */
-	struct tep_event_format *last_event;
-
-	char *trace_clock;
-};
-
-static inline void tep_set_flag(struct tep_handle *pevent, int flag)
-{
-	pevent->flags |= flag;
-}
-
-static inline unsigned short
-__tep_data2host2(struct tep_handle *pevent, unsigned short data)
-{
-	unsigned short swap;
-
-	if (pevent->host_bigendian == pevent->file_bigendian)
-		return data;
-
-	swap = ((data & 0xffULL) << 8) |
-		((data & (0xffULL << 8)) >> 8);
-
-	return swap;
-}
-
-static inline unsigned int
-__tep_data2host4(struct tep_handle *pevent, unsigned int data)
-{
-	unsigned int swap;
-
-	if (pevent->host_bigendian == pevent->file_bigendian)
-		return data;
-
-	swap = ((data & 0xffULL) << 24) |
-		((data & (0xffULL << 8)) << 8) |
-		((data & (0xffULL << 16)) >> 8) |
-		((data & (0xffULL << 24)) >> 24);
-
-	return swap;
-}
-
-static inline unsigned long long
-__tep_data2host8(struct tep_handle *pevent, unsigned long long data)
-{
-	unsigned long long swap;
-
-	if (pevent->host_bigendian == pevent->file_bigendian)
-		return data;
-
-	swap = ((data & 0xffULL) << 56) |
-		((data & (0xffULL << 8)) << 40) |
-		((data & (0xffULL << 16)) << 24) |
-		((data & (0xffULL << 24)) << 8) |
-		((data & (0xffULL << 32)) >> 8) |
-		((data & (0xffULL << 40)) >> 24) |
-		((data & (0xffULL << 48)) >> 40) |
-		((data & (0xffULL << 56)) >> 56);
-
-	return swap;
-}
-
-#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)					\
+#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);				\
+	__tep_data2host8(pevent, __val);			\
 })
 
 static inline int tep_host_bigendian(void)
@@ -655,11 +524,12 @@ unsigned long long tep_read_number(struct tep_handle *pevent, const void *ptr, i
 int tep_read_number_field(struct tep_format_field *field, const void *data,
 			  unsigned long long *value);
 
+struct tep_event_format *tep_get_first_event(struct tep_handle *tep);
+int tep_get_events_count(struct tep_handle *tep);
 struct tep_event_format *tep_find_event(struct tep_handle *pevent, int id);
 
 struct tep_event_format *
 tep_find_event_by_name(struct tep_handle *pevent, const char *sys, const char *name);
-
 struct tep_event_format *
 tep_find_event_by_record(struct tep_handle *pevent, struct tep_record *record);
 
@@ -689,65 +559,23 @@ struct tep_event_format **tep_list_events(struct tep_handle *pevent, enum tep_ev
 struct tep_format_field **tep_event_common_fields(struct tep_event_format *event);
 struct tep_format_field **tep_event_fields(struct tep_event_format *event);
 
-static inline int tep_get_cpus(struct tep_handle *pevent)
-{
-	return pevent->cpus;
-}
-
-static inline void tep_set_cpus(struct tep_handle *pevent, int cpus)
-{
-	pevent->cpus = cpus;
-}
-
-static inline int tep_get_long_size(struct tep_handle *pevent)
-{
-	return pevent->long_size;
-}
-
-static inline void tep_set_long_size(struct tep_handle *pevent, int long_size)
-{
-	pevent->long_size = long_size;
-}
-
-static inline int tep_get_page_size(struct tep_handle *pevent)
-{
-	return pevent->page_size;
-}
-
-static inline void tep_set_page_size(struct tep_handle *pevent, int _page_size)
-{
-	pevent->page_size = _page_size;
-}
-
-static inline int tep_is_file_bigendian(struct tep_handle *pevent)
-{
-	return pevent->file_bigendian;
-}
-
-static inline void tep_set_file_bigendian(struct tep_handle *pevent, int endian)
-{
-	pevent->file_bigendian = endian;
-}
-
-static inline int tep_is_host_bigendian(struct tep_handle *pevent)
-{
-	return pevent->host_bigendian;
-}
-
-static inline void tep_set_host_bigendian(struct tep_handle *pevent, int endian)
-{
-	pevent->host_bigendian = endian;
-}
-
-static inline int tep_is_latency_format(struct tep_handle *pevent)
-{
-	return pevent->latency_format;
-}
-
-static inline void tep_set_latency_format(struct tep_handle *pevent, int lat)
-{
-	pevent->latency_format = lat;
-}
+enum tep_endian {
+	TEP_LITTLE_ENDIAN = 0,
+	TEP_BIG_ENDIAN
+};
+int tep_get_cpus(struct tep_handle *pevent);
+void tep_set_cpus(struct tep_handle *pevent, int cpus);
+int tep_get_long_size(struct tep_handle *pevent);
+void tep_set_long_size(struct tep_handle *pevent, int long_size);
+int tep_get_page_size(struct tep_handle *pevent);
+void tep_set_page_size(struct tep_handle *pevent, int _page_size);
+int tep_is_file_bigendian(struct tep_handle *pevent);
+void tep_set_file_bigendian(struct tep_handle *pevent, enum tep_endian endian);
+int tep_is_host_bigendian(struct tep_handle *pevent);
+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);
 
 struct tep_handle *tep_alloc(void);
 void tep_free(struct tep_handle *pevent);
diff --git a/tools/lib/traceevent/event-plugin.c b/tools/lib/traceevent/event-plugin.c
index 46eb64eb0c2e..e74f16c88398 100644
--- a/tools/lib/traceevent/event-plugin.c
+++ b/tools/lib/traceevent/event-plugin.c
@@ -14,6 +14,7 @@
 #include <unistd.h>
 #include <dirent.h>
 #include "event-parse.h"
+#include "event-parse-local.h"
 #include "event-utils.h"
 #include "trace-seq.h"
 
diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c
index d64b6128fa7d..ed87cb56713d 100644
--- a/tools/lib/traceevent/parse-filter.c
+++ b/tools/lib/traceevent/parse-filter.c
@@ -11,6 +11,7 @@
 #include <sys/types.h>
 
 #include "event-parse.h"
+#include "event-parse-local.h"
 #include "event-utils.h"
 
 #define COMM "COMM"
diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
index a4d7de1c96d1..9b78160e23a2 100644
--- a/tools/perf/util/trace-event-parse.c
+++ b/tools/perf/util/trace-event-parse.c
@@ -37,10 +37,11 @@ static int get_common_field(struct scripting_context *context,
 	struct tep_format_field *field;
 
 	if (!*size) {
-		if (!pevent->events)
+
+		event = tep_get_first_event(pevent);
+		if (!event)
 			return 0;
 
-		event = pevent->events[0];
 		field = tep_find_common_field(event, type);
 		if (!field)
 			return 0;
@@ -192,25 +193,29 @@ struct tep_event_format *trace_find_next_event(struct tep_handle *pevent,
 					       struct tep_event_format *event)
 {
 	static int idx;
+	int events_count;
+	struct tep_event_format *all_events;
 
-	if (!pevent || !pevent->events)
+	all_events = tep_get_first_event(pevent);
+	events_count = tep_get_events_count(pevent);
+	if (!pevent || !all_events || events_count < 1)
 		return NULL;
 
 	if (!event) {
 		idx = 0;
-		return pevent->events[0];
+		return all_events;
 	}
 
-	if (idx < pevent->nr_events && event == pevent->events[idx]) {
+	if (idx < events_count && event == (all_events + idx)) {
 		idx++;
-		if (idx == pevent->nr_events)
+		if (idx == events_count)
 			return NULL;
-		return pevent->events[idx];
+		return (all_events + idx);
 	}
 
-	for (idx = 1; idx < pevent->nr_events; idx++) {
-		if (event == pevent->events[idx - 1])
-			return pevent->events[idx];
+	for (idx = 1; idx < events_count; idx++) {
+		if (event == (all_events + (idx - 1)))
+			return (all_events + idx);
 	}
 	return NULL;
 }
diff --git a/tools/perf/util/trace-event-read.c b/tools/perf/util/trace-event-read.c
index b98ee2a2eb44..cdf6de82a507 100644
--- a/tools/perf/util/trace-event-read.c
+++ b/tools/perf/util/trace-event-read.c
@@ -241,7 +241,7 @@ static int read_header_files(struct tep_handle *pevent)
 		 * The commit field in the page is of type long,
 		 * use that instead, since it represents the kernel.
 		 */
-		tep_set_long_size(pevent, pevent->header_page_size_size);
+		tep_set_long_size(pevent, tep_get_header_page_size(pevent));
 	}
 	free(header_page);
 
-- 
2.13.6

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

* [tip:perf/core] tools lib traceevent, perf tools: Move struct tep_handler definition in a local header file
  2018-10-05 16:22 [PATCH] tools/lib/traceevent, tools/perf: Move struct tep_handler definition in a local header file Steven Rostedt
  2018-10-05 16:28 ` Steven Rostedt
  2018-10-08 17:31 ` Arnaldo Carvalho de Melo
@ 2018-10-09  5:33 ` " tip-bot for Tzvetomir Stoyanov
  2019-07-26  3:58 ` [PATCH] tools/lib/traceevent, tools/perf: " Andres Freund
  3 siblings, 0 replies; 14+ messages in thread
From: tip-bot for Tzvetomir Stoyanov @ 2018-10-09  5:33 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: jolsa, linux-trace-devel, tglx, mingo, tstoyanov, rostedt,
	linux-kernel, acme, hpa, namhyung

Commit-ID:  bb3dd7e7c4d5e024d607c0ec06c2a2fb9408cc99
Gitweb:     https://git.kernel.org/tip/bb3dd7e7c4d5e024d607c0ec06c2a2fb9408cc99
Author:     Tzvetomir Stoyanov <tstoyanov@vmware.com>
AuthorDate: Fri, 5 Oct 2018 12:22:25 -0400
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 8 Oct 2018 15:05:37 -0300

tools lib traceevent, perf tools: Move struct tep_handler definition in a local header file

As traceevent is going to be transferred into a proper library,
its local data should be protected from the library users.
This patch encapsulates struct tep_handler into a local header,
not visible outside of the library. It implements also a bunch
of new APIs, which library users can use to access tep_handler members.

Signed-off-by: Tzvetomir Stoyanov <tstoyanov@vmware.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: linux trace devel <linux-trace-devel@vger.kernel.org>
Cc: tzvetomir stoyanov <tstoyanov@vmware.com>
Link: http://lkml.kernel.org/r/20181005122225.522155df@gandalf.local.home
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/traceevent/Build               |   1 +
 tools/lib/traceevent/event-parse-api.c   | 275 +++++++++++++++++++++++++++++++
 tools/lib/traceevent/event-parse-local.h |  92 +++++++++++
 tools/lib/traceevent/event-parse.c       |   2 +
 tools/lib/traceevent/event-parse.h       | 228 ++++---------------------
 tools/lib/traceevent/event-plugin.c      |   1 +
 tools/lib/traceevent/parse-filter.c      |   1 +
 tools/perf/util/trace-event-parse.c      |  25 +--
 tools/perf/util/trace-event-read.c       |   2 +-
 9 files changed, 416 insertions(+), 211 deletions(-)

diff --git a/tools/lib/traceevent/Build b/tools/lib/traceevent/Build
index 0050c145d806..ba54bfce0b0b 100644
--- a/tools/lib/traceevent/Build
+++ b/tools/lib/traceevent/Build
@@ -5,6 +5,7 @@ libtraceevent-y += parse-filter.o
 libtraceevent-y += parse-utils.o
 libtraceevent-y += kbuffer-parse.o
 libtraceevent-y += tep_strerror.o
+libtraceevent-y += event-parse-api.o
 
 plugin_jbd2-y         += plugin_jbd2.o
 plugin_hrtimer-y      += plugin_hrtimer.o
diff --git a/tools/lib/traceevent/event-parse-api.c b/tools/lib/traceevent/event-parse-api.c
new file mode 100644
index 000000000000..61f7149085ee
--- /dev/null
+++ b/tools/lib/traceevent/event-parse-api.c
@@ -0,0 +1,275 @@
+// SPDX-License-Identifier: LGPL-2.1
+/*
+ * Copyright (C) 2009, 2010 Red Hat Inc, Steven Rostedt <srostedt@redhat.com>
+ *
+ */
+
+#include "event-parse.h"
+#include "event-parse-local.h"
+#include "event-utils.h"
+
+/**
+ * tep_get_first_event - returns the first event in the events array
+ * @tep: a handle to the tep_handle
+ *
+ * This returns pointer to the first element of the events array
+ * If @tep is NULL, NULL is returned.
+ */
+struct tep_event_format *tep_get_first_event(struct tep_handle *tep)
+{
+	if (tep && tep->events)
+		return tep->events[0];
+
+	return NULL;
+}
+
+/**
+ * tep_get_events_count - get the number of defined events
+ * @tep: a handle to the tep_handle
+ *
+ * This returns number of elements in event array
+ * If @tep is NULL, 0 is returned.
+ */
+int tep_get_events_count(struct tep_handle *tep)
+{
+	if(tep)
+		return tep->nr_events;
+	return 0;
+}
+
+/**
+ * tep_set_flag - set event parser flag
+ * @tep: a handle to the tep_handle
+ * @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
+  */
+void tep_set_flag(struct tep_handle *tep, int flag)
+{
+	if(tep)
+		tep->flags |= flag;
+}
+
+unsigned short __tep_data2host2(struct tep_handle *pevent, unsigned short data)
+{
+	unsigned short swap;
+
+	if (!pevent || pevent->host_bigendian == pevent->file_bigendian)
+		return data;
+
+	swap = ((data & 0xffULL) << 8) |
+		((data & (0xffULL << 8)) >> 8);
+
+	return swap;
+}
+
+unsigned int __tep_data2host4(struct tep_handle *pevent, unsigned int data)
+{
+	unsigned int swap;
+
+	if (!pevent || pevent->host_bigendian == pevent->file_bigendian)
+		return data;
+
+	swap = ((data & 0xffULL) << 24) |
+		((data & (0xffULL << 8)) << 8) |
+		((data & (0xffULL << 16)) >> 8) |
+		((data & (0xffULL << 24)) >> 24);
+
+	return swap;
+}
+
+unsigned long long
+__tep_data2host8(struct tep_handle *pevent, unsigned long long data)
+{
+	unsigned long long swap;
+
+	if (!pevent || pevent->host_bigendian == pevent->file_bigendian)
+		return data;
+
+	swap = ((data & 0xffULL) << 56) |
+		((data & (0xffULL << 8)) << 40) |
+		((data & (0xffULL << 16)) << 24) |
+		((data & (0xffULL << 24)) << 8) |
+		((data & (0xffULL << 32)) >> 8) |
+		((data & (0xffULL << 40)) >> 24) |
+		((data & (0xffULL << 48)) >> 40) |
+		((data & (0xffULL << 56)) >> 56);
+
+	return swap;
+}
+
+/**
+ * tep_get_header_page_size - get size of the header page
+ * @pevent: a handle to the tep_handle
+ *
+ * This returns size of the header page
+ * If @pevent is NULL, 0 is returned.
+ */
+int tep_get_header_page_size(struct tep_handle *pevent)
+{
+	if(pevent)
+		return pevent->header_page_size_size;
+	return 0;
+}
+
+/**
+ * tep_get_cpus - get the number of CPUs
+ * @pevent: a handle to the tep_handle
+ *
+ * This returns the number of CPUs
+ * If @pevent is NULL, 0 is returned.
+ */
+int tep_get_cpus(struct tep_handle *pevent)
+{
+	if(pevent)
+		return pevent->cpus;
+	return 0;
+}
+
+/**
+ * tep_set_cpus - set the number of CPUs
+ * @pevent: a handle to the tep_handle
+ *
+ * This sets the number of CPUs
+ */
+void tep_set_cpus(struct tep_handle *pevent, int cpus)
+{
+	if(pevent)
+		pevent->cpus = cpus;
+}
+
+/**
+ * tep_get_long_size - get the size of a long integer on the current machine
+ * @pevent: a handle to the tep_handle
+ *
+ * This returns the size of a long integer on the current machine
+ * If @pevent is NULL, 0 is returned.
+ */
+int tep_get_long_size(struct tep_handle *pevent)
+{
+	if(pevent)
+		return pevent->long_size;
+	return 0;
+}
+
+/**
+ * tep_set_long_size - set the size of a long integer on the current 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
+ */
+void tep_set_long_size(struct tep_handle *pevent, int long_size)
+{
+	if(pevent)
+		pevent->long_size = long_size;
+}
+
+/**
+ * tep_get_page_size - get the size of a memory page on the current machine
+ * @pevent: a handle to the tep_handle
+ *
+ * This returns the size of a memory page on the current machine
+ * If @pevent is NULL, 0 is returned.
+ */
+int tep_get_page_size(struct tep_handle *pevent)
+{
+	if(pevent)
+		return pevent->page_size;
+	return 0;
+}
+
+/**
+ * tep_set_page_size - set the size of a memory page on the current 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
+ */
+void tep_set_page_size(struct tep_handle *pevent, int _page_size)
+{
+	if(pevent)
+		pevent->page_size = _page_size;
+}
+
+/**
+ * tep_is_file_bigendian - get if the file is in big endian order
+ * @pevent: a handle to the tep_handle
+ *
+ * This returns if the file is in big endian order
+ * If @pevent is NULL, 0 is returned.
+ */
+int tep_is_file_bigendian(struct tep_handle *pevent)
+{
+	if(pevent)
+		return pevent->file_bigendian;
+	return 0;
+}
+
+/**
+ * tep_set_file_bigendian - set if the file is in big endian order
+ * @pevent: a handle to the tep_handle
+ * @endian: non zero, if the file is in big endian order
+ *
+ * This sets if the file is in big endian order
+ */
+void tep_set_file_bigendian(struct tep_handle *pevent, enum tep_endian endian)
+{
+	if(pevent)
+		pevent->file_bigendian = endian;
+}
+
+/**
+ * tep_is_host_bigendian - get if the order of the current host is big endian
+ * @pevent: a handle to the tep_handle
+ *
+ * This gets if the order of the current host is big endian
+ * If @pevent is NULL, 0 is returned.
+ */
+int tep_is_host_bigendian(struct tep_handle *pevent)
+{
+	if(pevent)
+		return pevent->host_bigendian;
+	return 0;
+}
+
+/**
+ * tep_set_host_bigendian - set the order of the local host
+ * @pevent: a handle to the tep_handle
+ * @endian: non zero, if the local host has big endian order
+ *
+ * This sets the order of the local host
+ */
+void tep_set_host_bigendian(struct tep_handle *pevent, enum tep_endian endian)
+{
+	if(pevent)
+		pevent->host_bigendian = endian;
+}
+
+/**
+ * tep_is_latency_format - get if the latency output format is configured
+ * @pevent: a handle to the tep_handle
+ *
+ * This gets if the latency output format is configured
+ * If @pevent is NULL, 0 is returned.
+ */
+int tep_is_latency_format(struct tep_handle *pevent)
+{
+	if(pevent)
+		return pevent->latency_format;
+	return 0;
+}
+
+/**
+ * tep_set_latency_format - set the latency output format
+ * @pevent: a handle to the tep_handle
+ * @lat: non zero for latency output format
+ *
+ * This sets the latency output format
+  */
+void tep_set_latency_format(struct tep_handle *pevent, int lat)
+{
+	if(pevent)
+		pevent->latency_format = lat;
+}
diff --git a/tools/lib/traceevent/event-parse-local.h b/tools/lib/traceevent/event-parse-local.h
new file mode 100644
index 000000000000..b9bddde577f8
--- /dev/null
+++ b/tools/lib/traceevent/event-parse-local.h
@@ -0,0 +1,92 @@
+// SPDX-License-Identifier: LGPL-2.1
+/*
+ * Copyright (C) 2009, 2010 Red Hat Inc, Steven Rostedt <srostedt@redhat.com>
+ *
+ */
+
+#ifndef _PARSE_EVENTS_INT_H
+#define _PARSE_EVENTS_INT_H
+
+struct cmdline;
+struct cmdline_list;
+struct func_map;
+struct func_list;
+struct event_handler;
+struct func_resolver;
+
+struct tep_handle {
+	int ref_count;
+
+	int header_page_ts_offset;
+	int header_page_ts_size;
+	int header_page_size_offset;
+	int header_page_size_size;
+	int header_page_data_offset;
+	int header_page_data_size;
+	int header_page_overwrite;
+
+	enum tep_endian file_bigendian;
+	enum tep_endian host_bigendian;
+
+	int latency_format;
+
+	int old_format;
+
+	int cpus;
+	int long_size;
+	int page_size;
+
+	struct cmdline *cmdlines;
+	struct cmdline_list *cmdlist;
+	int cmdline_count;
+
+	struct func_map *func_map;
+	struct func_resolver *func_resolver;
+	struct func_list *funclist;
+	unsigned int func_count;
+
+	struct printk_map *printk_map;
+	struct printk_list *printklist;
+	unsigned int printk_count;
+
+
+	struct tep_event_format **events;
+	int nr_events;
+	struct tep_event_format **sort_events;
+	enum tep_event_sort_type last_type;
+
+	int type_offset;
+	int type_size;
+
+	int pid_offset;
+	int pid_size;
+
+	int pc_offset;
+	int pc_size;
+
+	int flags_offset;
+	int flags_size;
+
+	int ld_offset;
+	int ld_size;
+
+	int print_raw;
+
+	int test_filters;
+
+	int flags;
+
+	struct tep_format_field *bprint_ip_field;
+	struct tep_format_field *bprint_fmt_field;
+	struct tep_format_field *bprint_buf_field;
+
+	struct event_handler *handlers;
+	struct tep_function_handler *func_handlers;
+
+	/* cache */
+	struct tep_event_format *last_event;
+
+	char *trace_clock;
+};
+
+#endif /* _PARSE_EVENTS_INT_H */
diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 233179a712d6..3692f29fee46 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -22,6 +22,8 @@
 
 #include <netinet/in.h>
 #include "event-parse.h"
+
+#include "event-parse-local.h"
 #include "event-utils.h"
 #include "trace-seq.h"
 
diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index 9c29a5f7aa39..16bf4c890b6f 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -405,149 +405,18 @@ void tep_print_plugins(struct trace_seq *s,
 			const char *prefix, const char *suffix,
 			const struct tep_plugin_list *list);
 
-struct cmdline;
-struct cmdline_list;
-struct func_map;
-struct func_list;
-struct event_handler;
-struct func_resolver;
-
+/* tep_handle */
 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);
 
-struct tep_handle {
-	int ref_count;
-
-	int header_page_ts_offset;
-	int header_page_ts_size;
-	int header_page_size_offset;
-	int header_page_size_size;
-	int header_page_data_offset;
-	int header_page_data_size;
-	int header_page_overwrite;
-
-	int file_bigendian;
-	int host_bigendian;
-
-	int latency_format;
-
-	int old_format;
-
-	int cpus;
-	int long_size;
-	int page_size;
-
-	struct cmdline *cmdlines;
-	struct cmdline_list *cmdlist;
-	int cmdline_count;
-
-	struct func_map *func_map;
-	struct func_resolver *func_resolver;
-	struct func_list *funclist;
-	unsigned int func_count;
-
-	struct printk_map *printk_map;
-	struct printk_list *printklist;
-	unsigned int printk_count;
-
-
-	struct tep_event_format **events;
-	int nr_events;
-	struct tep_event_format **sort_events;
-	enum tep_event_sort_type last_type;
-
-	int type_offset;
-	int type_size;
-
-	int pid_offset;
-	int pid_size;
-
- 	int pc_offset;
-	int pc_size;
-
-	int flags_offset;
-	int flags_size;
-
-	int ld_offset;
-	int ld_size;
-
-	int print_raw;
-
-	int test_filters;
-
-	int flags;
-
-	struct tep_format_field *bprint_ip_field;
-	struct tep_format_field *bprint_fmt_field;
-	struct tep_format_field *bprint_buf_field;
-
-	struct event_handler *handlers;
-	struct tep_function_handler *func_handlers;
-
-	/* cache */
-	struct tep_event_format *last_event;
-
-	char *trace_clock;
-};
-
-static inline void tep_set_flag(struct tep_handle *pevent, int flag)
-{
-	pevent->flags |= flag;
-}
-
-static inline unsigned short
-__tep_data2host2(struct tep_handle *pevent, unsigned short data)
-{
-	unsigned short swap;
-
-	if (pevent->host_bigendian == pevent->file_bigendian)
-		return data;
-
-	swap = ((data & 0xffULL) << 8) |
-		((data & (0xffULL << 8)) >> 8);
-
-	return swap;
-}
-
-static inline unsigned int
-__tep_data2host4(struct tep_handle *pevent, unsigned int data)
-{
-	unsigned int swap;
-
-	if (pevent->host_bigendian == pevent->file_bigendian)
-		return data;
-
-	swap = ((data & 0xffULL) << 24) |
-		((data & (0xffULL << 8)) << 8) |
-		((data & (0xffULL << 16)) >> 8) |
-		((data & (0xffULL << 24)) >> 24);
-
-	return swap;
-}
-
-static inline unsigned long long
-__tep_data2host8(struct tep_handle *pevent, unsigned long long data)
-{
-	unsigned long long swap;
-
-	if (pevent->host_bigendian == pevent->file_bigendian)
-		return data;
-
-	swap = ((data & 0xffULL) << 56) |
-		((data & (0xffULL << 8)) << 40) |
-		((data & (0xffULL << 16)) << 24) |
-		((data & (0xffULL << 24)) << 8) |
-		((data & (0xffULL << 32)) >> 8) |
-		((data & (0xffULL << 40)) >> 24) |
-		((data & (0xffULL << 48)) >> 40) |
-		((data & (0xffULL << 56)) >> 56);
-
-	return swap;
-}
-
-#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)					\
+#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;				\
 								\
@@ -655,11 +524,12 @@ unsigned long long tep_read_number(struct tep_handle *pevent, const void *ptr, i
 int tep_read_number_field(struct tep_format_field *field, const void *data,
 			  unsigned long long *value);
 
+struct tep_event_format *tep_get_first_event(struct tep_handle *tep);
+int tep_get_events_count(struct tep_handle *tep);
 struct tep_event_format *tep_find_event(struct tep_handle *pevent, int id);
 
 struct tep_event_format *
 tep_find_event_by_name(struct tep_handle *pevent, const char *sys, const char *name);
-
 struct tep_event_format *
 tep_find_event_by_record(struct tep_handle *pevent, struct tep_record *record);
 
@@ -689,65 +559,23 @@ struct tep_event_format **tep_list_events(struct tep_handle *pevent, enum tep_ev
 struct tep_format_field **tep_event_common_fields(struct tep_event_format *event);
 struct tep_format_field **tep_event_fields(struct tep_event_format *event);
 
-static inline int tep_get_cpus(struct tep_handle *pevent)
-{
-	return pevent->cpus;
-}
-
-static inline void tep_set_cpus(struct tep_handle *pevent, int cpus)
-{
-	pevent->cpus = cpus;
-}
-
-static inline int tep_get_long_size(struct tep_handle *pevent)
-{
-	return pevent->long_size;
-}
-
-static inline void tep_set_long_size(struct tep_handle *pevent, int long_size)
-{
-	pevent->long_size = long_size;
-}
-
-static inline int tep_get_page_size(struct tep_handle *pevent)
-{
-	return pevent->page_size;
-}
-
-static inline void tep_set_page_size(struct tep_handle *pevent, int _page_size)
-{
-	pevent->page_size = _page_size;
-}
-
-static inline int tep_is_file_bigendian(struct tep_handle *pevent)
-{
-	return pevent->file_bigendian;
-}
-
-static inline void tep_set_file_bigendian(struct tep_handle *pevent, int endian)
-{
-	pevent->file_bigendian = endian;
-}
-
-static inline int tep_is_host_bigendian(struct tep_handle *pevent)
-{
-	return pevent->host_bigendian;
-}
-
-static inline void tep_set_host_bigendian(struct tep_handle *pevent, int endian)
-{
-	pevent->host_bigendian = endian;
-}
-
-static inline int tep_is_latency_format(struct tep_handle *pevent)
-{
-	return pevent->latency_format;
-}
-
-static inline void tep_set_latency_format(struct tep_handle *pevent, int lat)
-{
-	pevent->latency_format = lat;
-}
+enum tep_endian {
+        TEP_LITTLE_ENDIAN = 0,
+        TEP_BIG_ENDIAN
+};
+int tep_get_cpus(struct tep_handle *pevent);
+void tep_set_cpus(struct tep_handle *pevent, int cpus);
+int tep_get_long_size(struct tep_handle *pevent);
+void tep_set_long_size(struct tep_handle *pevent, int long_size);
+int tep_get_page_size(struct tep_handle *pevent);
+void tep_set_page_size(struct tep_handle *pevent, int _page_size);
+int tep_is_file_bigendian(struct tep_handle *pevent);
+void tep_set_file_bigendian(struct tep_handle *pevent, enum tep_endian endian);
+int tep_is_host_bigendian(struct tep_handle *pevent);
+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);
 
 struct tep_handle *tep_alloc(void);
 void tep_free(struct tep_handle *pevent);
diff --git a/tools/lib/traceevent/event-plugin.c b/tools/lib/traceevent/event-plugin.c
index 46eb64eb0c2e..e74f16c88398 100644
--- a/tools/lib/traceevent/event-plugin.c
+++ b/tools/lib/traceevent/event-plugin.c
@@ -14,6 +14,7 @@
 #include <unistd.h>
 #include <dirent.h>
 #include "event-parse.h"
+#include "event-parse-local.h"
 #include "event-utils.h"
 #include "trace-seq.h"
 
diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c
index d64b6128fa7d..ed87cb56713d 100644
--- a/tools/lib/traceevent/parse-filter.c
+++ b/tools/lib/traceevent/parse-filter.c
@@ -11,6 +11,7 @@
 #include <sys/types.h>
 
 #include "event-parse.h"
+#include "event-parse-local.h"
 #include "event-utils.h"
 
 #define COMM "COMM"
diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
index 02f97f5dd588..32e558a65af3 100644
--- a/tools/perf/util/trace-event-parse.c
+++ b/tools/perf/util/trace-event-parse.c
@@ -37,10 +37,11 @@ static int get_common_field(struct scripting_context *context,
 	struct tep_format_field *field;
 
 	if (!*size) {
-		if (!pevent->events)
+
+		event = tep_get_first_event(pevent);
+		if (!event)
 			return 0;
 
-		event = pevent->events[0];
 		field = tep_find_common_field(event, type);
 		if (!field)
 			return 0;
@@ -193,25 +194,29 @@ struct tep_event_format *trace_find_next_event(struct tep_handle *pevent,
 					       struct tep_event_format *event)
 {
 	static int idx;
+	int events_count;
+	struct tep_event_format *all_events;
 
-	if (!pevent || !pevent->events)
+	all_events = tep_get_first_event(pevent);
+	events_count = tep_get_events_count(pevent);
+	if (!pevent || !all_events || events_count < 1)
 		return NULL;
 
 	if (!event) {
 		idx = 0;
-		return pevent->events[0];
+		return all_events;
 	}
 
-	if (idx < pevent->nr_events && event == pevent->events[idx]) {
+	if (idx < events_count && event == (all_events + idx)) {
 		idx++;
-		if (idx == pevent->nr_events)
+		if (idx == events_count)
 			return NULL;
-		return pevent->events[idx];
+		return (all_events + idx);
 	}
 
-	for (idx = 1; idx < pevent->nr_events; idx++) {
-		if (event == pevent->events[idx - 1])
-			return pevent->events[idx];
+	for (idx = 1; idx < events_count; idx++) {
+		if (event == (all_events + (idx - 1)))
+			return (all_events + idx);
 	}
 	return NULL;
 }
diff --git a/tools/perf/util/trace-event-read.c b/tools/perf/util/trace-event-read.c
index add8441de579..76f12c705ef9 100644
--- a/tools/perf/util/trace-event-read.c
+++ b/tools/perf/util/trace-event-read.c
@@ -241,7 +241,7 @@ static int read_header_files(struct tep_handle *pevent)
 		 * The commit field in the page is of type long,
 		 * use that instead, since it represents the kernel.
 		 */
-		tep_set_long_size(pevent, pevent->header_page_size_size);
+		tep_set_long_size(pevent, tep_get_header_page_size(pevent));
 	}
 	free(header_page);
 

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

* Re: [PATCH] tools/lib/traceevent, tools/perf: Move struct tep_handler definition in a local header file
  2018-10-05 16:22 [PATCH] tools/lib/traceevent, tools/perf: Move struct tep_handler definition in a local header file Steven Rostedt
                   ` (2 preceding siblings ...)
  2018-10-09  5:33 ` [tip:perf/core] tools lib traceevent, perf tools: " tip-bot for Tzvetomir Stoyanov
@ 2019-07-26  3:58 ` " Andres Freund
  2019-07-26  8:25   ` Tzvetomir Stoyanov
  2019-07-26 13:12   ` Steven Rostedt
  3 siblings, 2 replies; 14+ messages in thread
From: Andres Freund @ 2019-07-26  3:58 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Arnaldo Carvalho de Melo, LKML, Linux Trace Devel,
	Tzvetomir Stoyanov, Jiri Olsa, Ingo Molnar, Namhyung Kim

Hi,

On 2018-10-05 12:22:25 -0400, Steven Rostedt wrote:
> From: Tzvetomir Stoyanov <tstoyanov@vmware.com>
>
> As traceevent is going to be transferred into a proper library,
> its local data should be protected from the library users.
> This patch encapsulates struct tep_handler into a local header,
> not visible outside of the library. It implements also a bunch
> of new APIs, which library users can use to access tep_handler members.

This commit appears to have broken perf script --gen-script /
trace_find_next_event().  As far as I can tell:

@ -192,25 +193,29 @@ struct tep_event_format *trace_find_next_event(struct tep_handle *pevent,
>  					       struct tep_event_format *event)
>  {
>  	static int idx;
> +	int events_count;
> +	struct tep_event_format *all_events;
>
> -	if (!pevent || !pevent->events)
> +	all_events = tep_get_first_event(pevent);
> +	events_count = tep_get_events_count(pevent);
> +	if (!pevent || !all_events || events_count < 1)
>  		return NULL;
>
>  	if (!event) {
>  		idx = 0;
> -		return pevent->events[0];
> +		return all_events;
>  	}
>
> -	if (idx < pevent->nr_events && event == pevent->events[idx]) {
> +	if (idx < events_count && event == (all_events + idx)) {
>  		idx++;
> -		if (idx == pevent->nr_events)
> +		if (idx == events_count)
>  			return NULL;
> -		return pevent->events[idx];
> +		return (all_events + idx);
>  	}
>
> -	for (idx = 1; idx < pevent->nr_events; idx++) {
> -		if (event == pevent->events[idx - 1])
> -			return pevent->events[idx];
> +	for (idx = 1; idx < events_count; idx++) {
> +		if (event == (all_events + (idx - 1)))
> +			return (all_events + idx);
>  	}
>  	return NULL;
>  }

Is just plain wrong, as:

> -		return pevent->events[idx];
> +		return (all_events + idx);

that's not a valid conversion. ->events isn't an array of tep_handle,
it's an array of tep_handle* (and even if it were, the previous notation
would have needed to dereference the value to make it comparable). What
this does is look idx behind the individually allocated event. Which is
incorrect.


To reproduce the crash, just generating a trace file with at least two
events suffices:

perf record -e raw_syscalls:sys_enter,raw_syscalls:sys_exit sleep 1

gdb --args perf script -g py

Program received signal SIGSEGV, Segmentation fault.
__strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:96
96	../sysdeps/x86_64/multiarch/strlen-avx2.S: No such file or directory.
(gdb) bt
#0  __strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:96
#1  0x00007ffff726f9ef in _IO_vfprintf_internal (s=s@entry=0x555555c23970, format=format@entry=0x5555558fed76 "def %s__%s(",
    ap=ap@entry=0x7fffffffb900) at vfprintf.c:1638
#2  0x00007ffff7326536 in ___fprintf_chk (fp=fp@entry=0x555555c23970, flag=flag@entry=1,
    format=format@entry=0x5555558fed76 "def %s__%s(") at fprintf_chk.c:35
#3  0x000055555587065c in fprintf (__fmt=0x5555558fed76 "def %s__%s(", __stream=0x555555c23970)
    at /usr/include/x86_64-linux-gnu/bits/stdio2.h:100
#4  python_generate_script (pevent=0x555555c2e620, outfile=<optimized out>)
    at util/scripting-engines/trace-event-python.c:1651
#5  0x0000555555745762 in cmd_script (argc=<optimized out>, argv=0x7fffffffe4d0) at builtin-script.c:3743
#6  0x000055555579821d in run_builtin (p=0x555555abeb38 <commands+408>, argc=3, argv=0x7fffffffe4d0) at perf.c:303
#7  0x0000555555712b7a in handle_internal_command (argv=0x7fffffffe4d0, argc=3) at perf.c:355
#8  run_argv (argcp=<synthetic pointer>, argv=<synthetic pointer>) at perf.c:399
#9  main (argc=3, argv=0x7fffffffe4d0) at perf.c:521


The fix (in recent kernel versions) appears to just bee to use
tep_get_event(), instead of the old pevent->events[...]. But it appears
to me that
commit 80c5526c8544ae76cba31fb9702ab8accac1f0f3
Author: Tzvetomir Stoyanov <tstoyanov@vmware.com>
Date:   2019-04-01 12:43:12 -0400

    tools lib traceevent: Implement new traceevent APIs for accessing struct tep_handler fields

ommitted adding it to event-parse.h. It appears to be intended as public
API however, given that it got documented in

commit 747e942c3925bb85e2865371664499a98fca83b0
Author: Tzvetomir Stoyanov <tstoyanov@vmware.com>
Date:   2019-05-10 15:56:23 -0400

    tools lib traceevent: Man pages for libtraceevent event get APIs


The following patch fixes this for me. I can polish it up, but I'm
wondering if I'm missing something here?

diff --git i/tools/lib/traceevent/event-parse.h w/tools/lib/traceevent/event-parse.h
index 642f68ab5fb2..7ebc9b5308d4 100644
--- i/tools/lib/traceevent/event-parse.h
+++ w/tools/lib/traceevent/event-parse.h
@@ -517,6 +517,7 @@ int tep_read_number_field(struct tep_format_field *field, const void *data,
 			  unsigned long long *value);
 
 struct tep_event *tep_get_first_event(struct tep_handle *tep);
+struct tep_event *tep_get_event(struct tep_handle *tep, int index);
 int tep_get_events_count(struct tep_handle *tep);
 struct tep_event *tep_find_event(struct tep_handle *tep, int id);
 
diff --git i/tools/perf/util/trace-event-parse.c w/tools/perf/util/trace-event-parse.c
index 62bc61155dd1..6a035ffd58ac 100644
--- i/tools/perf/util/trace-event-parse.c
+++ w/tools/perf/util/trace-event-parse.c
@@ -179,28 +179,26 @@ struct tep_event *trace_find_next_event(struct tep_handle *pevent,
 {
 	static int idx;
 	int events_count;
-	struct tep_event *all_events;
 
-	all_events = tep_get_first_event(pevent);
 	events_count = tep_get_events_count(pevent);
-	if (!pevent || !all_events || events_count < 1)
+	if (!pevent || events_count < 1)
 		return NULL;
 
 	if (!event) {
 		idx = 0;
-		return all_events;
+		return tep_get_event(pevent, 0);
 	}
 
-	if (idx < events_count && event == (all_events + idx)) {
+	if (idx < events_count && event == tep_get_event(pevent, idx)) {
 		idx++;
 		if (idx == events_count)
 			return NULL;
-		return (all_events + idx);
+		return tep_get_event(pevent, idx);
 	}
 
 	for (idx = 1; idx < events_count; idx++) {
-		if (event == (all_events + (idx - 1)))
-			return (all_events + idx);
+		if (event == tep_get_event(pevent, idx - 1))
+			return tep_get_event(pevent, idx);
 	}
 	return NULL;
 }




Not related to this crash, but it also seems that the whole
trace_find_next_event() API ought to be removed. Back when it was

-struct event *trace_find_next_event(struct event *event)
-{
-       if (!event)
-               return event_list;
-
-       return event->next;
-}

it made some sense, but the changes in

commit aaf045f72335653b24784d6042be8e4aee114403
Author: Steven Rostedt <srostedt@redhat.com>
Date:   2012-04-06 00:47:56 +0200

    perf: Have perf use the new libtraceevent.a library

seem to make the current API somewhat absurd, as evidenced by the
complexity in trace_find_next_event(). I mean even just removing that
function and changing the two callsites to simple for loops with
tep_get_events_count() & tep_get_event() ought to be a lot better.

Greetings,

Andres Freund

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

* Re: [PATCH] tools/lib/traceevent, tools/perf: Move struct tep_handler definition in a local header file
  2019-07-26  3:58 ` [PATCH] tools/lib/traceevent, tools/perf: " Andres Freund
@ 2019-07-26  8:25   ` Tzvetomir Stoyanov
  2019-07-26 13:12   ` Steven Rostedt
  1 sibling, 0 replies; 14+ messages in thread
From: Tzvetomir Stoyanov @ 2019-07-26  8:25 UTC (permalink / raw)
  To: Andres Freund
  Cc: Steven Rostedt, Arnaldo Carvalho de Melo, LKML,
	Linux Trace Devel, Jiri Olsa, Ingo Molnar, Namhyung Kim

Hi Andres,

On Fri, Jul 26, 2019 at 6:58 AM Andres Freund <andres@anarazel.de> wrote:
>
> Hi,
>
...
>
> Is just plain wrong, as:
>
> > -             return pevent->events[idx];
> > +             return (all_events + idx);
>
> that's not a valid conversion. ->events isn't an array of tep_handle,
> it's an array of tep_handle* (and even if it were, the previous notation
> would have needed to dereference the value to make it comparable). What
> this does is look idx behind the individually allocated event. Which is
> incorrect.
>
You are right, it is a bug.
>
...
>
> The following patch fixes this for me. I can polish it up, but I'm
> wondering if I'm missing something here?
>
> diff --git i/tools/lib/traceevent/event-parse.h w/tools/lib/traceevent/event-parse.h
> index 642f68ab5fb2..7ebc9b5308d4 100644
> --- i/tools/lib/traceevent/event-parse.h
> +++ w/tools/lib/traceevent/event-parse.h
> @@ -517,6 +517,7 @@ int tep_read_number_field(struct tep_format_field *field, const void *data,
>                           unsigned long long *value);
>
>  struct tep_event *tep_get_first_event(struct tep_handle *tep);
> +struct tep_event *tep_get_event(struct tep_handle *tep, int index);
>  int tep_get_events_count(struct tep_handle *tep);
>  struct tep_event *tep_find_event(struct tep_handle *tep, int id);
>
> diff --git i/tools/perf/util/trace-event-parse.c w/tools/perf/util/trace-event-parse.c
> index 62bc61155dd1..6a035ffd58ac 100644
> --- i/tools/perf/util/trace-event-parse.c
> +++ w/tools/perf/util/trace-event-parse.c
> @@ -179,28 +179,26 @@ struct tep_event *trace_find_next_event(struct tep_handle *pevent,
>  {
>         static int idx;
>         int events_count;
> -       struct tep_event *all_events;
>
> -       all_events = tep_get_first_event(pevent);
>         events_count = tep_get_events_count(pevent);
> -       if (!pevent || !all_events || events_count < 1)
> +       if (!pevent || events_count < 1)
>                 return NULL;
>
>         if (!event) {
>                 idx = 0;
> -               return all_events;
> +               return tep_get_event(pevent, 0);
>         }
>
> -       if (idx < events_count && event == (all_events + idx)) {
> +       if (idx < events_count && event == tep_get_event(pevent, idx)) {
>                 idx++;
>                 if (idx == events_count)
>                         return NULL;
> -               return (all_events + idx);
> +               return tep_get_event(pevent, idx);
>         }
>
>         for (idx = 1; idx < events_count; idx++) {
> -               if (event == (all_events + (idx - 1)))
> -                       return (all_events + idx);
> +               if (event == tep_get_event(pevent, idx - 1))
> +                       return tep_get_event(pevent, idx);
>         }
>         return NULL;
>  }
>
>

I'm OK with the first part of the patch, the changes in
tools/lib/traceevent/event-parse.h.
tep_get_event() is an API and is omitted in the header by mistake, it
should be there.

As for the fix for the crash itself, I think it would be better to
implement your suggestion -
removing trace_find_next_event() at all and replacing it with
tep_get_events_count() and
tep_get_event() loop.

>
>
> Not related to this crash, but it also seems that the whole
> trace_find_next_event() API ought to be removed. Back when it was
>
> -struct event *trace_find_next_event(struct event *event)
> -{
> -       if (!event)
> -               return event_list;
> -
> -       return event->next;
> -}
>
> it made some sense, but the changes in
>
> commit aaf045f72335653b24784d6042be8e4aee114403
> Author: Steven Rostedt <srostedt@redhat.com>
> Date:   2012-04-06 00:47:56 +0200
>
>     perf: Have perf use the new libtraceevent.a library
>
> seem to make the current API somewhat absurd, as evidenced by the
> complexity in trace_find_next_event(). I mean even just removing that
> function and changing the two callsites to simple for loops with
> tep_get_events_count() & tep_get_event() ought to be a lot better.
>
> Greetings,
>
> Andres Freund

Thanks!

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

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

* Re: [PATCH] tools/lib/traceevent, tools/perf: Move struct tep_handler definition in a local header file
  2019-07-26  3:58 ` [PATCH] tools/lib/traceevent, tools/perf: " Andres Freund
  2019-07-26  8:25   ` Tzvetomir Stoyanov
@ 2019-07-26 13:12   ` Steven Rostedt
  2019-07-26 20:55     ` Andres Freund
  1 sibling, 1 reply; 14+ messages in thread
From: Steven Rostedt @ 2019-07-26 13:12 UTC (permalink / raw)
  To: Andres Freund
  Cc: Arnaldo Carvalho de Melo, LKML, Linux Trace Devel,
	Tzvetomir Stoyanov, Jiri Olsa, Ingo Molnar, Namhyung Kim

On Thu, 25 Jul 2019 20:58:29 -0700
Andres Freund <andres@anarazel.de> wrote:

> Hi,
> 

Hi Andres,

> On 2018-10-05 12:22:25 -0400, Steven Rostedt wrote:
> > From: Tzvetomir Stoyanov <tstoyanov@vmware.com>
> >
> > As traceevent is going to be transferred into a proper library,
> > its local data should be protected from the library users.
> > This patch encapsulates struct tep_handler into a local header,
> > not visible outside of the library. It implements also a bunch
> > of new APIs, which library users can use to access tep_handler members.  
> 
> This commit appears to have broken perf script --gen-script /
> trace_find_next_event().  As far as I can tell:
> 
> @ -192,25 +193,29 @@ struct tep_event_format *trace_find_next_event(struct tep_handle *pevent,
> >  					       struct tep_event_format *event)
> >  {
> >  	static int idx;
> > +	int events_count;
> > +	struct tep_event_format *all_events;
> >
> > -	if (!pevent || !pevent->events)
> > +	all_events = tep_get_first_event(pevent);
> > +	events_count = tep_get_events_count(pevent);

> 
> Is just plain wrong, as:
> 
> > -		return pevent->events[idx];
> > +		return (all_events + idx);  
> 
> that's not a valid conversion. ->events isn't an array of tep_handle,
> it's an array of tep_handle* (and even if it were, the previous notation

You're right, it is wrong, but it's not tep_handle* but
tep_event_format*.

The tep_get_first_event() returns a pointer to the first event, but
that's not an array. We can't use that to index to other events.

> would have needed to dereference the value to make it comparable). What
> this does is look idx behind the individually allocated event. Which is
> incorrect.
> 
> 



> 
> The fix (in recent kernel versions) appears to just bee to use
> tep_get_event(), instead of the old pevent->events[...]. But it appears
> to me that
> commit 80c5526c8544ae76cba31fb9702ab8accac1f0f3
> Author: Tzvetomir Stoyanov <tstoyanov@vmware.com>
> Date:   2019-04-01 12:43:12 -0400
> 
>     tools lib traceevent: Implement new traceevent APIs for accessing struct tep_handler fields
> 
> ommitted adding it to event-parse.h. It appears to be intended as public
> API however, given that it got documented in

And it appears that we missed to add it ;-)

> 
> commit 747e942c3925bb85e2865371664499a98fca83b0
> Author: Tzvetomir Stoyanov <tstoyanov@vmware.com>
> Date:   2019-05-10 15:56:23 -0400
> 
>     tools lib traceevent: Man pages for libtraceevent event get APIs
> 
> 
> The following patch fixes this for me. I can polish it up, but I'm
> wondering if I'm missing something here?
> 
> diff --git i/tools/lib/traceevent/event-parse.h w/tools/lib/traceevent/event-parse.h
> index 642f68ab5fb2..7ebc9b5308d4 100644
> --- i/tools/lib/traceevent/event-parse.h
> +++ w/tools/lib/traceevent/event-parse.h
> @@ -517,6 +517,7 @@ int tep_read_number_field(struct tep_format_field *field, const void *data,
>  			  unsigned long long *value);
>  
>  struct tep_event *tep_get_first_event(struct tep_handle *tep);
> +struct tep_event *tep_get_event(struct tep_handle *tep, int index);

I was looking at the tep_get_event() code, and we should switch that to
"unsigned int index" otherwise passing in a negative number will return
an address outside the array.

>  int tep_get_events_count(struct tep_handle *tep);
>  struct tep_event *tep_find_event(struct tep_handle *tep, int id);
>  
> diff --git i/tools/perf/util/trace-event-parse.c w/tools/perf/util/trace-event-parse.c
> index 62bc61155dd1..6a035ffd58ac 100644
> --- i/tools/perf/util/trace-event-parse.c
> +++ w/tools/perf/util/trace-event-parse.c
> @@ -179,28 +179,26 @@ struct tep_event *trace_find_next_event(struct tep_handle *pevent,
>  {
>  	static int idx;
>  	int events_count;
> -	struct tep_event *all_events;
>  
> -	all_events = tep_get_first_event(pevent);
>  	events_count = tep_get_events_count(pevent);

I think we can get rid of the events_count and all its checks, as the
same check is done within tep_get_event().

> -	if (!pevent || !all_events || events_count < 1)
> +	if (!pevent || events_count < 1)

	if (!pevent)

>  		return NULL;
>  
>  	if (!event) {
>  		idx = 0;
> -		return all_events;
> +		return tep_get_event(pevent, 0);
>  	}
>  
> -	if (idx < events_count && event == (all_events + idx)) {
> +	if (idx < events_count && event == tep_get_event(pevent, idx)) {

	if (event == tep_get_event(pevent, idx))
		return tep_get_event(pevent, ++idx);

>  		idx++;
>  		if (idx == events_count)
>  			return NULL;
> -		return (all_events + idx);
> +		return tep_get_event(pevent, idx);
>  	}
>  

	struct tep_event_format *next_event;

	for (idx = 0; next_event = tep_get_event(pevent, idx); idx++)
		if (event == next_event)
			return tep_get_event(pevent, ++idx);

Also, I think setting the idx to 1 in the loop is wrong. Why? think of
this:

	first_event = trace_find_next_event(pevent, NULL);

	next_event = trace_find_next_event(pevent, first_event);
	for (i = 0; i < 5; i++)
		next_event = trace_find_next_event(pevent, next_event);

	second_event = trace_find_next_event(pevent, first_event);

second_event would become NULL.

Care to send a formal patch?

Thanks!

-- Steve


>  	for (idx = 1; idx < events_count; idx++) {
> -		if (event == (all_events + (idx - 1)))
> -			return (all_events + idx);
> +		if (event == tep_get_event(pevent, idx - 1))
> +			return tep_get_event(pevent, idx);
>  	}

>  	return NULL;
>  }
> 
> 
> 
> 
> Not related to this crash, but it also seems that the whole
> trace_find_next_event() API ought to be removed. Back when it was
> 
> -struct event *trace_find_next_event(struct event *event)
> -{
> -       if (!event)
> -               return event_list;
> -
> -       return event->next;
> -}
> 
> it made some sense, but the changes in
> 
> commit aaf045f72335653b24784d6042be8e4aee114403
> Author: Steven Rostedt <srostedt@redhat.com>
> Date:   2012-04-06 00:47:56 +0200
> 
>     perf: Have perf use the new libtraceevent.a library
> 
> seem to make the current API somewhat absurd, as evidenced by the
> complexity in trace_find_next_event(). I mean even just removing that
> function and changing the two callsites to simple for loops with
> tep_get_events_count() & tep_get_event() ought to be a lot better.
> 
> Greetings,
> 
> Andres Freund


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

* Re: [PATCH] tools/lib/traceevent, tools/perf: Move struct tep_handler definition in a local header file
  2019-07-26 13:12   ` Steven Rostedt
@ 2019-07-26 20:55     ` Andres Freund
  2019-07-26 21:00       ` Steven Rostedt
  2019-08-01 14:09       ` Steven Rostedt
  0 siblings, 2 replies; 14+ messages in thread
From: Andres Freund @ 2019-07-26 20:55 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Arnaldo Carvalho de Melo, LKML, Linux Trace Devel,
	Tzvetomir Stoyanov, Jiri Olsa, Ingo Molnar, Namhyung Kim

Hi,

On 2019-07-26 09:12:00 -0400, Steven Rostedt wrote:
> On Thu, 25 Jul 2019 20:58:29 -0700
> Andres Freund <andres@anarazel.de> wrote:
> > 
> > Is just plain wrong, as:
> > 
> > > -		return pevent->events[idx];
> > > +		return (all_events + idx);  
> > 
> > that's not a valid conversion. ->events isn't an array of tep_handle,
> > it's an array of tep_handle* (and even if it were, the previous notation
> 
> You're right, it is wrong, but it's not tep_handle* but
> tep_event_format*.

Err, yea. Typo.



> > diff --git i/tools/lib/traceevent/event-parse.h w/tools/lib/traceevent/event-parse.h
> > index 642f68ab5fb2..7ebc9b5308d4 100644
> > --- i/tools/lib/traceevent/event-parse.h
> > +++ w/tools/lib/traceevent/event-parse.h
> > @@ -517,6 +517,7 @@ int tep_read_number_field(struct tep_format_field *field, const void *data,
> >  			  unsigned long long *value);
> >  
> >  struct tep_event *tep_get_first_event(struct tep_handle *tep);
> > +struct tep_event *tep_get_event(struct tep_handle *tep, int index);
> 
> I was looking at the tep_get_event() code, and we should switch that to
> "unsigned int index" otherwise passing in a negative number will return
> an address outside the array.

Makes sense.


> >  int tep_get_events_count(struct tep_handle *tep);
> >  struct tep_event *tep_find_event(struct tep_handle *tep, int id);
> >  
> > diff --git i/tools/perf/util/trace-event-parse.c w/tools/perf/util/trace-event-parse.c
> > index 62bc61155dd1..6a035ffd58ac 100644
> > --- i/tools/perf/util/trace-event-parse.c
> > +++ w/tools/perf/util/trace-event-parse.c
> > @@ -179,28 +179,26 @@ struct tep_event *trace_find_next_event(struct tep_handle *pevent,
> >  {
> >  	static int idx;
> >  	int events_count;
> > -	struct tep_event *all_events;
> >  
> > -	all_events = tep_get_first_event(pevent);
> >  	events_count = tep_get_events_count(pevent);
> 
> I think we can get rid of the events_count and all its checks, as the
> same check is done within tep_get_event().

> > -	if (!pevent || !all_events || events_count < 1)
> > +	if (!pevent || events_count < 1)
> 
> 	if (!pevent)
> 
> >  		return NULL;
> >  
> >  	if (!event) {
> >  		idx = 0;
> > -		return all_events;
> > +		return tep_get_event(pevent, 0);
> >  	}
> >  
> > -	if (idx < events_count && event == (all_events + idx)) {
> > +	if (idx < events_count && event == tep_get_event(pevent, idx)) {
> 
> 	if (event == tep_get_event(pevent, idx))
> 		return tep_get_event(pevent, ++idx);
> 
> >  		idx++;
> >  		if (idx == events_count)
> >  			return NULL;
> > -		return (all_events + idx);
> > +		return tep_get_event(pevent, idx);
> >  	}
> >  
> 
> 	struct tep_event_format *next_event;
> 
> 	for (idx = 0; next_event = tep_get_event(pevent, idx); idx++)
> 		if (event == next_event)
> 			return tep_get_event(pevent, ++idx);
> 
> Also, I think setting the idx to 1 in the loop is wrong. Why? think of
> this:
> 
> 	first_event = trace_find_next_event(pevent, NULL);
> 
> 	next_event = trace_find_next_event(pevent, first_event);
> 	for (i = 0; i < 5; i++)
> 		next_event = trace_find_next_event(pevent, next_event);
> 
> 	second_event = trace_find_next_event(pevent, first_event);
> 
> second_event would become NULL.

How about my proposal to instead change the loops in
trace-event-{python,perl}.c, the only callers of trace_find_next_event,
to be something akin to

[idx_type_for_tep_get_event] event_count = tep_get_events_count(pevent);
for ([idx_type_for_tep_get_event] idx = 0; idx < event_count; idx++)
{
	struct tep_event *event = tep_get_events(...);

}

and just removing trace_find_next_event()? It's not a nice API imo, and
seems unnecessary given that the events aren't a linked list anymore.


> Care to send a formal patch?

Will do.

Greetings,

Andres Freund

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

* Re: [PATCH] tools/lib/traceevent, tools/perf: Move struct tep_handler definition in a local header file
  2019-07-26 20:55     ` Andres Freund
@ 2019-07-26 21:00       ` Steven Rostedt
  2019-08-01 14:09       ` Steven Rostedt
  1 sibling, 0 replies; 14+ messages in thread
From: Steven Rostedt @ 2019-07-26 21:00 UTC (permalink / raw)
  To: Andres Freund
  Cc: Arnaldo Carvalho de Melo, LKML, Linux Trace Devel,
	Tzvetomir Stoyanov, Jiri Olsa, Ingo Molnar, Namhyung Kim

On Fri, 26 Jul 2019 13:55:44 -0700
Andres Freund <andres@anarazel.de> wrote:

> How about my proposal to instead change the loops in
> trace-event-{python,perl}.c, the only callers of trace_find_next_event,
> to be something akin to
> 
> [idx_type_for_tep_get_event] event_count = tep_get_events_count(pevent);
> for ([idx_type_for_tep_get_event] idx = 0; idx < event_count; idx++)
> {
> 	struct tep_event *event = tep_get_events(...);
> 
> }
> 
> and just removing trace_find_next_event()? It's not a nice API imo, and
> seems unnecessary given that the events aren't a linked list anymore.

Yep, go for it :-)


> 
> 
> > Care to send a formal patch?  
> 
> Will do.

Thanks!

-- Steve

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

* Re: [PATCH] tools/lib/traceevent, tools/perf: Move struct tep_handler definition in a local header file
  2019-07-26 20:55     ` Andres Freund
  2019-07-26 21:00       ` Steven Rostedt
@ 2019-08-01 14:09       ` Steven Rostedt
  1 sibling, 0 replies; 14+ messages in thread
From: Steven Rostedt @ 2019-08-01 14:09 UTC (permalink / raw)
  To: Andres Freund
  Cc: Arnaldo Carvalho de Melo, LKML, Linux Trace Devel,
	Tzvetomir Stoyanov, Jiri Olsa, Ingo Molnar, Namhyung Kim

On Fri, 26 Jul 2019 13:55:44 -0700
Andres Freund <andres@anarazel.de> wrote:

> > Care to send a formal patch?  
> 
> Will do.


Hi Andres,

Have you had a chance to send a patch?

Thanks!

-- Steve

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

* Re: [PATCH] tools/lib/traceevent, tools/perf: Move struct tep_handler definition in a local header file
  2018-08-13 15:06 Tzvetomir Stoyanov (VMware)
@ 2018-10-03  0:05 ` Steven Rostedt
  0 siblings, 0 replies; 14+ messages in thread
From: Steven Rostedt @ 2018-10-03  0:05 UTC (permalink / raw)
  To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel


Actually, I have comments about this before I'll pull it in. Thus, just
fold the second patch into this one and resubmit a v2.

On Mon, 13 Aug 2018 18:06:03 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> In order to make libtraceevent into a proper library, variables, data
> structures and functions require a unique prefix to prevent name space
> conflicts.

OK, we already did the name space changes, this patch doesn't have
anything to do with it. The above can be eliminated from the patch.

> This moves definition of struct tep_handler in a local header.
> It implements also a bunch of new APIs, which can be used to access
> tep_handler members

Need to explain why we need this. I don't actually remember the full
rationale :-). If you do, please add it here. A change log should not
only say what it is doing, but why it is doing it. As someone said in
another patch on LKML, "why did you take the effort to write this patch"

> 
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
>  tools/lib/traceevent/Build               |   1 +
>  tools/lib/traceevent/event-parse-api.c   | 295 +++++++++++++++++++++++
>  tools/lib/traceevent/event-parse-local.h | 105 ++++++++
>  tools/lib/traceevent/event-parse.c       |   2 +
>  tools/lib/traceevent/event-parse.h       | 224 ++---------------
>  tools/lib/traceevent/event-plugin.c      |   1 +
>  tools/lib/traceevent/parse-filter.c      |   1 +
>  tools/perf/util/trace-event-parse.c      |  25 +-
>  tools/perf/util/trace-event-read.c       |   2 +-
>  9 files changed, 445 insertions(+), 211 deletions(-)
>  create mode 100644 tools/lib/traceevent/event-parse-api.c
>  create mode 100644 tools/lib/traceevent/event-parse-local.h
> 
> diff --git a/tools/lib/traceevent/Build b/tools/lib/traceevent/Build
> index c681d0575d16..c10a937cc85a 100644
> --- a/tools/lib/traceevent/Build
> +++ b/tools/lib/traceevent/Build
> @@ -4,6 +4,7 @@ libtraceevent-y += trace-seq.o
>  libtraceevent-y += parse-filter.o
>  libtraceevent-y += parse-utils.o
>  libtraceevent-y += kbuffer-parse.o
> +libtraceevent-y += event-parse-api.o
>  
>  plugin_jbd2-y         += plugin_jbd2.o
>  plugin_hrtimer-y      += plugin_hrtimer.o
> diff --git a/tools/lib/traceevent/event-parse-api.c b/tools/lib/traceevent/event-parse-api.c
> new file mode 100644
> index 000000000000..774d3dcc4909
> --- /dev/null
> +++ b/tools/lib/traceevent/event-parse-api.c
> @@ -0,0 +1,295 @@
> +/*
> + * Copyright (C) 2009, 2010 Red Hat Inc, Steven Rostedt <srostedt@redhat.com>
> + *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation;
> + * version 2.1 of the License (not later!)
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this program; if not,  see <http://www.gnu.org/licenses>
> + *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + *
> + *  The parts for function graph printing was taken and modified from the
> + *  Linux Kernel that were written by
> + *    - Copyright (C) 2009  Frederic Weisbecker,
> + *  Frederic Weisbecker gave his permission to relicense the code to
> + *  the Lesser General Public License.

The above comment about Frederic can be removed. This code doesn't
pertain to the function graph tracer from the kernel.


> + */
> +
> +#include "event-parse.h"
> +#include "event-parse-local.h"
> +#include "event-utils.h"
> +
> +/**
> + * tep_get_events - get events array
> + * @pevent: a handle to the tep_handle
> + *
> + * This returns pointer to the first element of the events array
> + * If @pevent is NULL, NULL is returned.
> + */
> +struct tep_event_format *tep_get_events(struct tep_handle *pevent)
> +{
> +	if(pevent && pevent->events)
> +		return pevent->events[0];

This is actually returning the first event in the events array.

Thus a more accurate name is:

 tep_get_first_event()


> +
> +	return NULL;
> +}
> +
> +/**
> + * tep_get_events_count - get events count

			- get the number of defined events

> + * @pevent: a handle to the tep_handle
> + *
> + * This returns number of elements in event array
> + * If @pevent is NULL, 0 is returned.
> + */
> +int tep_get_events_count(struct tep_handle *pevent)
> +{
> +	if(pevent)
> +		return pevent->nr_events;
> +	return 0;
> +}
> +
> +/**
> + * tep_set_flag - set event parser flag
> + * @pevent: a handle to the tep_handle

Also, if we are going to start creating new functions, lets start using
a different name for the tep_handle, instead of using "pevent" lets use
"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
> +  */
> +void tep_set_flag(struct tep_handle *pevent, int flag)
> +{
> +	if(pevent)
> +		pevent->flags |= flag;
> +}
> +
> +unsigned short __tep_data2host2(struct tep_handle *pevent, unsigned short data)
> +{
> +	unsigned short swap;
> +
> +	if (!pevent || pevent->host_bigendian == pevent->file_bigendian)
> +		return data;
> +
> +	swap = ((data & 0xffULL) << 8) |
> +		((data & (0xffULL << 8)) >> 8);
> +
> +	return swap;
> +}
> +
> +unsigned int __tep_data2host4(struct tep_handle *pevent, unsigned int data)
> +{
> +	unsigned int swap;
> +
> +	if (!pevent || pevent->host_bigendian == pevent->file_bigendian)
> +		return data;
> +
> +	swap = ((data & 0xffULL) << 24) |
> +		((data & (0xffULL << 8)) << 8) |
> +		((data & (0xffULL << 16)) >> 8) |
> +		((data & (0xffULL << 24)) >> 24);
> +
> +	return swap;
> +}
> +
> +unsigned long long
> +__tep_data2host8(struct tep_handle *pevent, unsigned long long data)
> +{
> +	unsigned long long swap;
> +
> +	if (!pevent || pevent->host_bigendian == pevent->file_bigendian)
> +		return data;
> +
> +	swap = ((data & 0xffULL) << 56) |
> +		((data & (0xffULL << 8)) << 40) |
> +		((data & (0xffULL << 16)) << 24) |
> +		((data & (0xffULL << 24)) << 8) |
> +		((data & (0xffULL << 32)) >> 8) |
> +		((data & (0xffULL << 40)) >> 24) |
> +		((data & (0xffULL << 48)) >> 40) |
> +		((data & (0xffULL << 56)) >> 56);
> +
> +	return swap;
> +}
> +
> +/**
> + * tep_get_header_page_size - get size of the header page
> + * @pevent: a handle to the tep_handle
> + *
> + * This returns size of the header page
> + * If @pevent is NULL, 0 is returned.
> + */
> +int tep_get_header_page_size(struct tep_handle *pevent)
> +{
> +	if(pevent)
> +		return pevent->header_page_size_size;
> +	return 0;
> +}
> +
> +/**
> + * tep_get_cpus - get the number of CPUs
> + * @pevent: a handle to the tep_handle
> + *
> + * This returns the number of CPUs
> + * If @pevent is NULL, 0 is returned.
> + */
> +int tep_get_cpus(struct tep_handle *pevent)
> +{
> +	if(pevent)
> +		return pevent->cpus;
> +	return 0;
> +}
> +
> +/**
> + * tep_set_cpus - set the number of CPUs
> + * @pevent: a handle to the tep_handle
> + *
> + * This sets the number of CPUs
> + */
> +void tep_set_cpus(struct tep_handle *pevent, int cpus)
> +{
> +	if(pevent)
> +		pevent->cpus = cpus;
> +}
> +
> +/**
> + * tep_get_long_size - get the size of a long integer on the current machine
> + * @pevent: a handle to the tep_handle
> + *
> + * This returns the size of a long integer on the current machine
> + * If @pevent is NULL, 0 is returned.
> + */
> +int tep_get_long_size(struct tep_handle *pevent)
> +{
> +	if(pevent)
> +		return pevent->long_size;
> +	return 0;
> +}
> +
> +/**
> + * tep_set_long_size - set the size of a long integer on the current 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
> + */
> +void tep_set_long_size(struct tep_handle *pevent, int long_size)
> +{
> +	if(pevent)
> +		pevent->long_size = long_size;
> +}

Hmm, the get/set long size, lets leave it. I think it can be removed.
I'll investigate this further.

> +
> +/**
> + * tep_get_page_size - get the size of a memory page on the current machine
> + * @pevent: a handle to the tep_handle
> + *
> + * This returns the size of a memory page on the current machine
> + * If @pevent is NULL, 0 is returned.
> + */
> +int tep_get_page_size(struct tep_handle *pevent)
> +{
> +	if(pevent)
> +		return pevent->page_size;
> +	return 0;
> +}
> +
> +/**
> + * tep_set_page_size - set the size of a memory page on the current 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
> + */
> +void tep_set_page_size(struct tep_handle *pevent, int _page_size)
> +{
> +	if(pevent)
> +		pevent->page_size = _page_size;
> +}
> +
> +/**
> + * tep_is_file_bigendian - get if the file is in big endian order
> + * @pevent: a handle to the tep_handle
> + *
> + * This returns if the file is in big endian order
> + * If @pevent is NULL, 0 is returned.
> + */
> +int tep_is_file_bigendian(struct tep_handle *pevent)
> +{
> +	if(pevent)
> +		return pevent->file_bigendian;
> +	return 0;
> +}
> +
> +/**
> + * tep_set_file_bigendian - set if the file is in big endian order
> + * @pevent: a handle to the tep_handle
> + * @endian: non zero, if the file is in big endian order

We probably should make that an enum:

enum tep_endian {
	TEP_LITTLE_ENDIAN,
	TEP_BIG_ENDIAN,
};

And we wouldn't really need to change anything else:


> + *
> + * This sets if the file is in big endian order
> + */
> +void tep_set_file_bigendian(struct tep_handle *pevent, int endian)

				enum tep_endian endian)

> +{
> +	if(pevent)
> +		pevent->file_bigendian = endian;

And TEP_LITTLE_ENDIAN would be zero, and BIG_ENDIAN would be 1.

> +}
> +
> +/**
> + * tep_is_host_bigendian - get if the order of the current host is big endian
> + * @pevent: a handle to the tep_handle
> + *
> + * This gets if the order of the current host is big endian
> + * If @pevent is NULL, 0 is returned.
> + */
> +int tep_is_host_bigendian(struct tep_handle *pevent)
> +{
> +	if(pevent)
> +		return pevent->host_bigendian;
> +	return 0;
> +}
> +
> +/**
> + * tep_set_host_bigendian - set the order of the local host
> + * @pevent: a handle to the tep_handle
> + * @endian: non zero, if the local host has big endian order
> + *
> + * This sets the order of the local host
> + */
> +void tep_set_host_bigendian(struct tep_handle *pevent, int endian)
> +{
> +	if(pevent)
> +		pevent->host_bigendian = endian;
> +}
> +
> +/**
> + * tep_is_latency_format - get if the latency output format is configured
> + * @pevent: a handle to the tep_handle
> + *
> + * This gets if the latency output format is configured
> + * If @pevent is NULL, 0 is returned.
> + */
> +int tep_is_latency_format(struct tep_handle *pevent)
> +{
> +	if(pevent)
> +		return pevent->latency_format;
> +	return 0;
> +}
> +
> +/**
> + * tep_set_latency_format - set the latency output format
> + * @pevent: a handle to the tep_handle
> + * @lat: non zero for latency output format
> + *
> + * This sets the latency output format
> +  */
> +void tep_set_latency_format(struct tep_handle *pevent, int lat)
> +{
> +	if(pevent)
> +		pevent->latency_format = lat;

Hmm, I wonder if this should be made into flags. It's just a flag that
we print out as latency.


> +}
> diff --git a/tools/lib/traceevent/event-parse-local.h b/tools/lib/traceevent/event-parse-local.h
> new file mode 100644
> index 000000000000..a2414e7f3f78
> --- /dev/null
> +++ b/tools/lib/traceevent/event-parse-local.h
> @@ -0,0 +1,105 @@
> +/*
> + * Copyright (C) 2009, 2010 Red Hat Inc, Steven Rostedt <srostedt@redhat.com>
> + *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation;
> + * version 2.1 of the License (not later!)
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this program; if not,  see <http://www.gnu.org/licenses>
> + *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + */
> +#ifndef _PARSE_EVENTS_INT_H
> +#define _PARSE_EVENTS_INT_H
> +
> +struct cmdline;
> +struct cmdline_list;
> +struct func_map;
> +struct func_list;
> +struct event_handler;
> +struct func_resolver;
> +
> +struct tep_handle {
> +	int ref_count;
> +
> +	int header_page_ts_offset;
> +	int header_page_ts_size;
> +	int header_page_size_offset;
> +	int header_page_size_size;
> +	int header_page_data_offset;
> +	int header_page_data_size;
> +	int header_page_overwrite;
> +
> +	int file_bigendian;
> +	int host_bigendian;
> +
> +	int latency_format;
> +
> +	int old_format;
> +
> +	int cpus;
> +	int long_size;
> +	int page_size;
> +
> +	struct cmdline *cmdlines;
> +	struct cmdline_list *cmdlist;
> +	int cmdline_count;
> +
> +	struct func_map *func_map;
> +	struct func_resolver *func_resolver;
> +	struct func_list *funclist;
> +	unsigned int func_count;
> +
> +	struct printk_map *printk_map;
> +	struct printk_list *printklist;
> +	unsigned int printk_count;
> +
> +
> +	struct tep_event_format **events;
> +	int nr_events;
> +	struct tep_event_format **sort_events;
> +	enum tep_event_sort_type last_type;
> +
> +	int type_offset;
> +	int type_size;
> +
> +	int pid_offset;
> +	int pid_size;
> +
> + 	int pc_offset;
> +	int pc_size;
> +
> +	int flags_offset;
> +	int flags_size;
> +
> +	int ld_offset;
> +	int ld_size;
> +
> +	int print_raw;
> +
> +	int test_filters;
> +
> +	int flags;
> +
> +	struct tep_format_field *bprint_ip_field;
> +	struct tep_format_field *bprint_fmt_field;
> +	struct tep_format_field *bprint_buf_field;
> +
> +	struct event_handler *handlers;
> +	struct tep_function_handler *func_handlers;
> +
> +	/* cache */
> +	struct tep_event_format *last_event;
> +
> +	char *trace_clock;
> +};
> +
> +#endif /* _PARSE_EVENTS_INT_H */
> diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
> index 29e0825fbb92..aabd7516bafd 100644
> --- a/tools/lib/traceevent/event-parse.c
> +++ b/tools/lib/traceevent/event-parse.c
> @@ -37,6 +37,8 @@
>  
>  #include <netinet/in.h>
>  #include "event-parse.h"
> +

Why the added space?

If anything, a space should go before "event-parse.h"

-- Steve

> +#include "event-parse-local.h"
>  #include "event-utils.h"
>  
>  static const char *input_buf;
> diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
> index c8d911bed12a..90082df52505 100644
> --- a/tools/lib/traceevent/event-parse.h
> +++ b/tools/lib/traceevent/event-parse.h
> @@ -447,149 +447,18 @@ void tep_print_plugins(struct trace_seq *s,
>  			const char *prefix, const char *suffix,
>  			const struct tep_plugin_list *list);
>  
> -struct cmdline;
> -struct cmdline_list;
> -struct func_map;
> -struct func_list;
> -struct event_handler;
> -struct func_resolver;
> -
> +/* tep_handle */
>  typedef char *(tep_func_resolver_t)(void *priv,
>  				    unsigned long long *addrp, char **modp);
> +void tep_set_flag(struct tep_handle *pevent, 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);
>  
> -struct tep_handle {
> -	int ref_count;
> -
> -	int header_page_ts_offset;
> -	int header_page_ts_size;
> -	int header_page_size_offset;
> -	int header_page_size_size;
> -	int header_page_data_offset;
> -	int header_page_data_size;
> -	int header_page_overwrite;
> -
> -	int file_bigendian;
> -	int host_bigendian;
> -
> -	int latency_format;
> -
> -	int old_format;
> -
> -	int cpus;
> -	int long_size;
> -	int page_size;
> -
> -	struct cmdline *cmdlines;
> -	struct cmdline_list *cmdlist;
> -	int cmdline_count;
> -
> -	struct func_map *func_map;
> -	struct func_resolver *func_resolver;
> -	struct func_list *funclist;
> -	unsigned int func_count;
> -
> -	struct printk_map *printk_map;
> -	struct printk_list *printklist;
> -	unsigned int printk_count;
> -
> -
> -	struct tep_event_format **events;
> -	int nr_events;
> -	struct tep_event_format **sort_events;
> -	enum tep_event_sort_type last_type;
> -
> -	int type_offset;
> -	int type_size;
> -
> -	int pid_offset;
> -	int pid_size;
> -
> - 	int pc_offset;
> -	int pc_size;
> -
> -	int flags_offset;
> -	int flags_size;
> -
> -	int ld_offset;
> -	int ld_size;
> -
> -	int print_raw;
> -
> -	int test_filters;
> -
> -	int flags;
> -
> -	struct tep_format_field *bprint_ip_field;
> -	struct tep_format_field *bprint_fmt_field;
> -	struct tep_format_field *bprint_buf_field;
> -
> -	struct event_handler *handlers;
> -	struct tep_function_handler *func_handlers;
> -
> -	/* cache */
> -	struct tep_event_format *last_event;
> -
> -	char *trace_clock;
> -};
> -
> -static inline void tep_set_flag(struct tep_handle *pevent, int flag)
> -{
> -	pevent->flags |= flag;
> -}
> -
> -static inline unsigned short
> -__tep_data2host2(struct tep_handle *pevent, unsigned short data)
> -{
> -	unsigned short swap;
> -
> -	if (pevent->host_bigendian == pevent->file_bigendian)
> -		return data;
> -
> -	swap = ((data & 0xffULL) << 8) |
> -		((data & (0xffULL << 8)) >> 8);
> -
> -	return swap;
> -}
> -
> -static inline unsigned int
> -__tep_data2host4(struct tep_handle *pevent, unsigned int data)
> -{
> -	unsigned int swap;
> -
> -	if (pevent->host_bigendian == pevent->file_bigendian)
> -		return data;
> -
> -	swap = ((data & 0xffULL) << 24) |
> -		((data & (0xffULL << 8)) << 8) |
> -		((data & (0xffULL << 16)) >> 8) |
> -		((data & (0xffULL << 24)) >> 24);
> -
> -	return swap;
> -}
> -
> -static inline unsigned long long
> -__tep_data2host8(struct tep_handle *pevent, unsigned long long data)
> -{
> -	unsigned long long swap;
> -
> -	if (pevent->host_bigendian == pevent->file_bigendian)
> -		return data;
> -
> -	swap = ((data & 0xffULL) << 56) |
> -		((data & (0xffULL << 8)) << 40) |
> -		((data & (0xffULL << 16)) << 24) |
> -		((data & (0xffULL << 24)) << 8) |
> -		((data & (0xffULL << 32)) >> 8) |
> -		((data & (0xffULL << 40)) >> 24) |
> -		((data & (0xffULL << 48)) >> 40) |
> -		((data & (0xffULL << 56)) >> 56);
> -
> -	return swap;
> -}
> -
> -#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)					\
> +#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;				\
>  								\
> @@ -697,11 +566,12 @@ unsigned long long tep_read_number(struct tep_handle *pevent, const void *ptr, i
>  int tep_read_number_field(struct tep_format_field *field, const void *data,
>  			  unsigned long long *value);
>  
> +struct tep_event_format *tep_get_events(struct tep_handle *pevent);
> +int tep_get_events_count(struct tep_handle *pevent);
>  struct tep_event_format *tep_find_event(struct tep_handle *pevent, int id);
>  
>  struct tep_event_format *
>  tep_find_event_by_name(struct tep_handle *pevent, const char *sys, const char *name);
> -
>  struct tep_event_format *
>  tep_find_event_by_record(struct tep_handle *pevent, struct tep_record *record);
>  
> @@ -731,65 +601,19 @@ struct tep_event_format **tep_list_events(struct tep_handle *pevent, enum tep_ev
>  struct tep_format_field **tep_event_common_fields(struct tep_event_format *event);
>  struct tep_format_field **tep_event_fields(struct tep_event_format *event);
>  
> -static inline int tep_get_cpus(struct tep_handle *pevent)
> -{
> -	return pevent->cpus;
> -}
> -
> -static inline void tep_set_cpus(struct tep_handle *pevent, int cpus)
> -{
> -	pevent->cpus = cpus;
> -}
> -
> -static inline int tep_get_long_size(struct tep_handle *pevent)
> -{
> -	return pevent->long_size;
> -}
> -
> -static inline void tep_set_long_size(struct tep_handle *pevent, int long_size)
> -{
> -	pevent->long_size = long_size;
> -}
> -
> -static inline int tep_get_page_size(struct tep_handle *pevent)
> -{
> -	return pevent->page_size;
> -}
> -
> -static inline void tep_set_page_size(struct tep_handle *pevent, int _page_size)
> -{
> -	pevent->page_size = _page_size;
> -}
> -
> -static inline int tep_is_file_bigendian(struct tep_handle *pevent)
> -{
> -	return pevent->file_bigendian;
> -}
> -
> -static inline void tep_set_file_bigendian(struct tep_handle *pevent, int endian)
> -{
> -	pevent->file_bigendian = endian;
> -}
> -
> -static inline int tep_is_host_bigendian(struct tep_handle *pevent)
> -{
> -	return pevent->host_bigendian;
> -}
> -
> -static inline void tep_set_host_bigendian(struct tep_handle *pevent, int endian)
> -{
> -	pevent->host_bigendian = endian;
> -}
> -
> -static inline int tep_is_latency_format(struct tep_handle *pevent)
> -{
> -	return pevent->latency_format;
> -}
> -
> -static inline void tep_set_latency_format(struct tep_handle *pevent, int lat)
> -{
> -	pevent->latency_format = lat;
> -}
> +int tep_get_cpus(struct tep_handle *pevent);
> +void tep_set_cpus(struct tep_handle *pevent, int cpus);
> +int tep_get_long_size(struct tep_handle *pevent);
> +void tep_set_long_size(struct tep_handle *pevent, int long_size);
> +int tep_get_page_size(struct tep_handle *pevent);
> +void tep_set_page_size(struct tep_handle *pevent, int _page_size);
> +int tep_is_file_bigendian(struct tep_handle *pevent);
> +void tep_set_file_bigendian(struct tep_handle *pevent, int endian);
> +int tep_is_host_bigendian(struct tep_handle *pevent);
> +void tep_set_host_bigendian(struct tep_handle *pevent, int 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);
>  
>  struct tep_handle *tep_alloc(void);
>  void tep_free(struct tep_handle *pevent);
> diff --git a/tools/lib/traceevent/event-plugin.c b/tools/lib/traceevent/event-plugin.c
> index f6a9f9363c8d..d15313dbcc8f 100644
> --- a/tools/lib/traceevent/event-plugin.c
> +++ b/tools/lib/traceevent/event-plugin.c
> @@ -28,6 +28,7 @@
>  #include <unistd.h>
>  #include <dirent.h>
>  #include "event-parse.h"
> +#include "event-parse-local.h"
>  #include "event-utils.h"
>  
>  #define LOCAL_PLUGIN_DIR ".traceevent/plugins"
> diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c
> index c4a5404f5f2b..eebbff7cd1a3 100644
> --- a/tools/lib/traceevent/parse-filter.c
> +++ b/tools/lib/traceevent/parse-filter.c
> @@ -25,6 +25,7 @@
>  #include <sys/types.h>
>  
>  #include "event-parse.h"
> +#include "event-parse-local.h"
>  #include "event-utils.h"
>  
>  #define COMM "COMM"
> diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
> index 2ea658268767..dba0bf56a170 100644
> --- a/tools/perf/util/trace-event-parse.c
> +++ b/tools/perf/util/trace-event-parse.c
> @@ -37,10 +37,11 @@ static int get_common_field(struct scripting_context *context,
>  	struct tep_format_field *field;
>  
>  	if (!*size) {
> -		if (!pevent->events)
> +
> +		event = tep_get_events(pevent);
> +		if (!event)
>  			return 0;
>  
> -		event = pevent->events[0];
>  		field = tep_find_common_field(event, type);
>  		if (!field)
>  			return 0;
> @@ -193,25 +194,29 @@ struct tep_event_format *trace_find_next_event(struct tep_handle *pevent,
>  					       struct tep_event_format *event)
>  {
>  	static int idx;
> +	int events_count;
> +	struct tep_event_format *all_events;
>  
> -	if (!pevent || !pevent->events)
> +	all_events = tep_get_events(pevent);
> +	events_count = tep_get_events_count(pevent);
> +	if (!pevent || !all_events || events_count < 1)
>  		return NULL;
>  
>  	if (!event) {
>  		idx = 0;
> -		return pevent->events[0];
> +		return all_events;
>  	}
>  
> -	if (idx < pevent->nr_events && event == pevent->events[idx]) {
> +	if (idx < events_count && event == (all_events + idx)) {
>  		idx++;
> -		if (idx == pevent->nr_events)
> +		if (idx == events_count)
>  			return NULL;
> -		return pevent->events[idx];
> +		return (all_events + idx);
>  	}
>  
> -	for (idx = 1; idx < pevent->nr_events; idx++) {
> -		if (event == pevent->events[idx - 1])
> -			return pevent->events[idx];
> +	for (idx = 1; idx < events_count; idx++) {
> +		if (event == (all_events + (idx - 1)))
> +			return (all_events + idx);
>  	}
>  	return NULL;
>  }
> diff --git a/tools/perf/util/trace-event-read.c b/tools/perf/util/trace-event-read.c
> index b98ee2a2eb44..cdf6de82a507 100644
> --- a/tools/perf/util/trace-event-read.c
> +++ b/tools/perf/util/trace-event-read.c
> @@ -241,7 +241,7 @@ static int read_header_files(struct tep_handle *pevent)
>  		 * The commit field in the page is of type long,
>  		 * use that instead, since it represents the kernel.
>  		 */
> -		tep_set_long_size(pevent, pevent->header_page_size_size);
> +		tep_set_long_size(pevent, tep_get_header_page_size(pevent));
>  	}
>  	free(header_page);
>  

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

* [PATCH] tools/lib/traceevent, tools/perf: Move struct tep_handler definition in a local header file
@ 2018-08-13 15:06 Tzvetomir Stoyanov (VMware)
  2018-10-03  0:05 ` Steven Rostedt
  0 siblings, 1 reply; 14+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2018-08-13 15:06 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel, Tzvetomir Stoyanov (VMware)

In order to make libtraceevent into a proper library, variables, data
structures and functions require a unique prefix to prevent name space
conflicts. This moves definition of struct tep_handler in a local header.
It implements also a bunch of new APIs, which can be used to access
tep_handler members

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 tools/lib/traceevent/Build               |   1 +
 tools/lib/traceevent/event-parse-api.c   | 295 +++++++++++++++++++++++
 tools/lib/traceevent/event-parse-local.h | 105 ++++++++
 tools/lib/traceevent/event-parse.c       |   2 +
 tools/lib/traceevent/event-parse.h       | 224 ++---------------
 tools/lib/traceevent/event-plugin.c      |   1 +
 tools/lib/traceevent/parse-filter.c      |   1 +
 tools/perf/util/trace-event-parse.c      |  25 +-
 tools/perf/util/trace-event-read.c       |   2 +-
 9 files changed, 445 insertions(+), 211 deletions(-)
 create mode 100644 tools/lib/traceevent/event-parse-api.c
 create mode 100644 tools/lib/traceevent/event-parse-local.h

diff --git a/tools/lib/traceevent/Build b/tools/lib/traceevent/Build
index c681d0575d16..c10a937cc85a 100644
--- a/tools/lib/traceevent/Build
+++ b/tools/lib/traceevent/Build
@@ -4,6 +4,7 @@ libtraceevent-y += trace-seq.o
 libtraceevent-y += parse-filter.o
 libtraceevent-y += parse-utils.o
 libtraceevent-y += kbuffer-parse.o
+libtraceevent-y += event-parse-api.o
 
 plugin_jbd2-y         += plugin_jbd2.o
 plugin_hrtimer-y      += plugin_hrtimer.o
diff --git a/tools/lib/traceevent/event-parse-api.c b/tools/lib/traceevent/event-parse-api.c
new file mode 100644
index 000000000000..774d3dcc4909
--- /dev/null
+++ b/tools/lib/traceevent/event-parse-api.c
@@ -0,0 +1,295 @@
+/*
+ * Copyright (C) 2009, 2010 Red Hat Inc, Steven Rostedt <srostedt@redhat.com>
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation;
+ * version 2.1 of the License (not later!)
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this program; if not,  see <http://www.gnu.org/licenses>
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ *  The parts for function graph printing was taken and modified from the
+ *  Linux Kernel that were written by
+ *    - Copyright (C) 2009  Frederic Weisbecker,
+ *  Frederic Weisbecker gave his permission to relicense the code to
+ *  the Lesser General Public License.
+ */
+
+#include "event-parse.h"
+#include "event-parse-local.h"
+#include "event-utils.h"
+
+/**
+ * tep_get_events - get events array
+ * @pevent: a handle to the tep_handle
+ *
+ * This returns pointer to the first element of the events array
+ * If @pevent is NULL, NULL is returned.
+ */
+struct tep_event_format *tep_get_events(struct tep_handle *pevent)
+{
+	if(pevent && pevent->events)
+		return pevent->events[0];
+
+	return NULL;
+}
+
+/**
+ * tep_get_events_count - get events count
+ * @pevent: a handle to the tep_handle
+ *
+ * This returns number of elements in event array
+ * If @pevent is NULL, 0 is returned.
+ */
+int tep_get_events_count(struct tep_handle *pevent)
+{
+	if(pevent)
+		return pevent->nr_events;
+	return 0;
+}
+
+/**
+ * tep_set_flag - set event parser flag
+ * @pevent: a handle to the tep_handle
+ * @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
+  */
+void tep_set_flag(struct tep_handle *pevent, int flag)
+{
+	if(pevent)
+		pevent->flags |= flag;
+}
+
+unsigned short __tep_data2host2(struct tep_handle *pevent, unsigned short data)
+{
+	unsigned short swap;
+
+	if (!pevent || pevent->host_bigendian == pevent->file_bigendian)
+		return data;
+
+	swap = ((data & 0xffULL) << 8) |
+		((data & (0xffULL << 8)) >> 8);
+
+	return swap;
+}
+
+unsigned int __tep_data2host4(struct tep_handle *pevent, unsigned int data)
+{
+	unsigned int swap;
+
+	if (!pevent || pevent->host_bigendian == pevent->file_bigendian)
+		return data;
+
+	swap = ((data & 0xffULL) << 24) |
+		((data & (0xffULL << 8)) << 8) |
+		((data & (0xffULL << 16)) >> 8) |
+		((data & (0xffULL << 24)) >> 24);
+
+	return swap;
+}
+
+unsigned long long
+__tep_data2host8(struct tep_handle *pevent, unsigned long long data)
+{
+	unsigned long long swap;
+
+	if (!pevent || pevent->host_bigendian == pevent->file_bigendian)
+		return data;
+
+	swap = ((data & 0xffULL) << 56) |
+		((data & (0xffULL << 8)) << 40) |
+		((data & (0xffULL << 16)) << 24) |
+		((data & (0xffULL << 24)) << 8) |
+		((data & (0xffULL << 32)) >> 8) |
+		((data & (0xffULL << 40)) >> 24) |
+		((data & (0xffULL << 48)) >> 40) |
+		((data & (0xffULL << 56)) >> 56);
+
+	return swap;
+}
+
+/**
+ * tep_get_header_page_size - get size of the header page
+ * @pevent: a handle to the tep_handle
+ *
+ * This returns size of the header page
+ * If @pevent is NULL, 0 is returned.
+ */
+int tep_get_header_page_size(struct tep_handle *pevent)
+{
+	if(pevent)
+		return pevent->header_page_size_size;
+	return 0;
+}
+
+/**
+ * tep_get_cpus - get the number of CPUs
+ * @pevent: a handle to the tep_handle
+ *
+ * This returns the number of CPUs
+ * If @pevent is NULL, 0 is returned.
+ */
+int tep_get_cpus(struct tep_handle *pevent)
+{
+	if(pevent)
+		return pevent->cpus;
+	return 0;
+}
+
+/**
+ * tep_set_cpus - set the number of CPUs
+ * @pevent: a handle to the tep_handle
+ *
+ * This sets the number of CPUs
+ */
+void tep_set_cpus(struct tep_handle *pevent, int cpus)
+{
+	if(pevent)
+		pevent->cpus = cpus;
+}
+
+/**
+ * tep_get_long_size - get the size of a long integer on the current machine
+ * @pevent: a handle to the tep_handle
+ *
+ * This returns the size of a long integer on the current machine
+ * If @pevent is NULL, 0 is returned.
+ */
+int tep_get_long_size(struct tep_handle *pevent)
+{
+	if(pevent)
+		return pevent->long_size;
+	return 0;
+}
+
+/**
+ * tep_set_long_size - set the size of a long integer on the current 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
+ */
+void tep_set_long_size(struct tep_handle *pevent, int long_size)
+{
+	if(pevent)
+		pevent->long_size = long_size;
+}
+
+/**
+ * tep_get_page_size - get the size of a memory page on the current machine
+ * @pevent: a handle to the tep_handle
+ *
+ * This returns the size of a memory page on the current machine
+ * If @pevent is NULL, 0 is returned.
+ */
+int tep_get_page_size(struct tep_handle *pevent)
+{
+	if(pevent)
+		return pevent->page_size;
+	return 0;
+}
+
+/**
+ * tep_set_page_size - set the size of a memory page on the current 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
+ */
+void tep_set_page_size(struct tep_handle *pevent, int _page_size)
+{
+	if(pevent)
+		pevent->page_size = _page_size;
+}
+
+/**
+ * tep_is_file_bigendian - get if the file is in big endian order
+ * @pevent: a handle to the tep_handle
+ *
+ * This returns if the file is in big endian order
+ * If @pevent is NULL, 0 is returned.
+ */
+int tep_is_file_bigendian(struct tep_handle *pevent)
+{
+	if(pevent)
+		return pevent->file_bigendian;
+	return 0;
+}
+
+/**
+ * tep_set_file_bigendian - set if the file is in big endian order
+ * @pevent: a handle to the tep_handle
+ * @endian: non zero, if the file is in big endian order
+ *
+ * This sets if the file is in big endian order
+ */
+void tep_set_file_bigendian(struct tep_handle *pevent, int endian)
+{
+	if(pevent)
+		pevent->file_bigendian = endian;
+}
+
+/**
+ * tep_is_host_bigendian - get if the order of the current host is big endian
+ * @pevent: a handle to the tep_handle
+ *
+ * This gets if the order of the current host is big endian
+ * If @pevent is NULL, 0 is returned.
+ */
+int tep_is_host_bigendian(struct tep_handle *pevent)
+{
+	if(pevent)
+		return pevent->host_bigendian;
+	return 0;
+}
+
+/**
+ * tep_set_host_bigendian - set the order of the local host
+ * @pevent: a handle to the tep_handle
+ * @endian: non zero, if the local host has big endian order
+ *
+ * This sets the order of the local host
+ */
+void tep_set_host_bigendian(struct tep_handle *pevent, int endian)
+{
+	if(pevent)
+		pevent->host_bigendian = endian;
+}
+
+/**
+ * tep_is_latency_format - get if the latency output format is configured
+ * @pevent: a handle to the tep_handle
+ *
+ * This gets if the latency output format is configured
+ * If @pevent is NULL, 0 is returned.
+ */
+int tep_is_latency_format(struct tep_handle *pevent)
+{
+	if(pevent)
+		return pevent->latency_format;
+	return 0;
+}
+
+/**
+ * tep_set_latency_format - set the latency output format
+ * @pevent: a handle to the tep_handle
+ * @lat: non zero for latency output format
+ *
+ * This sets the latency output format
+  */
+void tep_set_latency_format(struct tep_handle *pevent, int lat)
+{
+	if(pevent)
+		pevent->latency_format = lat;
+}
diff --git a/tools/lib/traceevent/event-parse-local.h b/tools/lib/traceevent/event-parse-local.h
new file mode 100644
index 000000000000..a2414e7f3f78
--- /dev/null
+++ b/tools/lib/traceevent/event-parse-local.h
@@ -0,0 +1,105 @@
+/*
+ * Copyright (C) 2009, 2010 Red Hat Inc, Steven Rostedt <srostedt@redhat.com>
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation;
+ * version 2.1 of the License (not later!)
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this program; if not,  see <http://www.gnu.org/licenses>
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ */
+#ifndef _PARSE_EVENTS_INT_H
+#define _PARSE_EVENTS_INT_H
+
+struct cmdline;
+struct cmdline_list;
+struct func_map;
+struct func_list;
+struct event_handler;
+struct func_resolver;
+
+struct tep_handle {
+	int ref_count;
+
+	int header_page_ts_offset;
+	int header_page_ts_size;
+	int header_page_size_offset;
+	int header_page_size_size;
+	int header_page_data_offset;
+	int header_page_data_size;
+	int header_page_overwrite;
+
+	int file_bigendian;
+	int host_bigendian;
+
+	int latency_format;
+
+	int old_format;
+
+	int cpus;
+	int long_size;
+	int page_size;
+
+	struct cmdline *cmdlines;
+	struct cmdline_list *cmdlist;
+	int cmdline_count;
+
+	struct func_map *func_map;
+	struct func_resolver *func_resolver;
+	struct func_list *funclist;
+	unsigned int func_count;
+
+	struct printk_map *printk_map;
+	struct printk_list *printklist;
+	unsigned int printk_count;
+
+
+	struct tep_event_format **events;
+	int nr_events;
+	struct tep_event_format **sort_events;
+	enum tep_event_sort_type last_type;
+
+	int type_offset;
+	int type_size;
+
+	int pid_offset;
+	int pid_size;
+
+ 	int pc_offset;
+	int pc_size;
+
+	int flags_offset;
+	int flags_size;
+
+	int ld_offset;
+	int ld_size;
+
+	int print_raw;
+
+	int test_filters;
+
+	int flags;
+
+	struct tep_format_field *bprint_ip_field;
+	struct tep_format_field *bprint_fmt_field;
+	struct tep_format_field *bprint_buf_field;
+
+	struct event_handler *handlers;
+	struct tep_function_handler *func_handlers;
+
+	/* cache */
+	struct tep_event_format *last_event;
+
+	char *trace_clock;
+};
+
+#endif /* _PARSE_EVENTS_INT_H */
diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 29e0825fbb92..aabd7516bafd 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -37,6 +37,8 @@
 
 #include <netinet/in.h>
 #include "event-parse.h"
+
+#include "event-parse-local.h"
 #include "event-utils.h"
 
 static const char *input_buf;
diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index c8d911bed12a..90082df52505 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -447,149 +447,18 @@ void tep_print_plugins(struct trace_seq *s,
 			const char *prefix, const char *suffix,
 			const struct tep_plugin_list *list);
 
-struct cmdline;
-struct cmdline_list;
-struct func_map;
-struct func_list;
-struct event_handler;
-struct func_resolver;
-
+/* tep_handle */
 typedef char *(tep_func_resolver_t)(void *priv,
 				    unsigned long long *addrp, char **modp);
+void tep_set_flag(struct tep_handle *pevent, 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);
 
-struct tep_handle {
-	int ref_count;
-
-	int header_page_ts_offset;
-	int header_page_ts_size;
-	int header_page_size_offset;
-	int header_page_size_size;
-	int header_page_data_offset;
-	int header_page_data_size;
-	int header_page_overwrite;
-
-	int file_bigendian;
-	int host_bigendian;
-
-	int latency_format;
-
-	int old_format;
-
-	int cpus;
-	int long_size;
-	int page_size;
-
-	struct cmdline *cmdlines;
-	struct cmdline_list *cmdlist;
-	int cmdline_count;
-
-	struct func_map *func_map;
-	struct func_resolver *func_resolver;
-	struct func_list *funclist;
-	unsigned int func_count;
-
-	struct printk_map *printk_map;
-	struct printk_list *printklist;
-	unsigned int printk_count;
-
-
-	struct tep_event_format **events;
-	int nr_events;
-	struct tep_event_format **sort_events;
-	enum tep_event_sort_type last_type;
-
-	int type_offset;
-	int type_size;
-
-	int pid_offset;
-	int pid_size;
-
- 	int pc_offset;
-	int pc_size;
-
-	int flags_offset;
-	int flags_size;
-
-	int ld_offset;
-	int ld_size;
-
-	int print_raw;
-
-	int test_filters;
-
-	int flags;
-
-	struct tep_format_field *bprint_ip_field;
-	struct tep_format_field *bprint_fmt_field;
-	struct tep_format_field *bprint_buf_field;
-
-	struct event_handler *handlers;
-	struct tep_function_handler *func_handlers;
-
-	/* cache */
-	struct tep_event_format *last_event;
-
-	char *trace_clock;
-};
-
-static inline void tep_set_flag(struct tep_handle *pevent, int flag)
-{
-	pevent->flags |= flag;
-}
-
-static inline unsigned short
-__tep_data2host2(struct tep_handle *pevent, unsigned short data)
-{
-	unsigned short swap;
-
-	if (pevent->host_bigendian == pevent->file_bigendian)
-		return data;
-
-	swap = ((data & 0xffULL) << 8) |
-		((data & (0xffULL << 8)) >> 8);
-
-	return swap;
-}
-
-static inline unsigned int
-__tep_data2host4(struct tep_handle *pevent, unsigned int data)
-{
-	unsigned int swap;
-
-	if (pevent->host_bigendian == pevent->file_bigendian)
-		return data;
-
-	swap = ((data & 0xffULL) << 24) |
-		((data & (0xffULL << 8)) << 8) |
-		((data & (0xffULL << 16)) >> 8) |
-		((data & (0xffULL << 24)) >> 24);
-
-	return swap;
-}
-
-static inline unsigned long long
-__tep_data2host8(struct tep_handle *pevent, unsigned long long data)
-{
-	unsigned long long swap;
-
-	if (pevent->host_bigendian == pevent->file_bigendian)
-		return data;
-
-	swap = ((data & 0xffULL) << 56) |
-		((data & (0xffULL << 8)) << 40) |
-		((data & (0xffULL << 16)) << 24) |
-		((data & (0xffULL << 24)) << 8) |
-		((data & (0xffULL << 32)) >> 8) |
-		((data & (0xffULL << 40)) >> 24) |
-		((data & (0xffULL << 48)) >> 40) |
-		((data & (0xffULL << 56)) >> 56);
-
-	return swap;
-}
-
-#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)					\
+#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;				\
 								\
@@ -697,11 +566,12 @@ unsigned long long tep_read_number(struct tep_handle *pevent, const void *ptr, i
 int tep_read_number_field(struct tep_format_field *field, const void *data,
 			  unsigned long long *value);
 
+struct tep_event_format *tep_get_events(struct tep_handle *pevent);
+int tep_get_events_count(struct tep_handle *pevent);
 struct tep_event_format *tep_find_event(struct tep_handle *pevent, int id);
 
 struct tep_event_format *
 tep_find_event_by_name(struct tep_handle *pevent, const char *sys, const char *name);
-
 struct tep_event_format *
 tep_find_event_by_record(struct tep_handle *pevent, struct tep_record *record);
 
@@ -731,65 +601,19 @@ struct tep_event_format **tep_list_events(struct tep_handle *pevent, enum tep_ev
 struct tep_format_field **tep_event_common_fields(struct tep_event_format *event);
 struct tep_format_field **tep_event_fields(struct tep_event_format *event);
 
-static inline int tep_get_cpus(struct tep_handle *pevent)
-{
-	return pevent->cpus;
-}
-
-static inline void tep_set_cpus(struct tep_handle *pevent, int cpus)
-{
-	pevent->cpus = cpus;
-}
-
-static inline int tep_get_long_size(struct tep_handle *pevent)
-{
-	return pevent->long_size;
-}
-
-static inline void tep_set_long_size(struct tep_handle *pevent, int long_size)
-{
-	pevent->long_size = long_size;
-}
-
-static inline int tep_get_page_size(struct tep_handle *pevent)
-{
-	return pevent->page_size;
-}
-
-static inline void tep_set_page_size(struct tep_handle *pevent, int _page_size)
-{
-	pevent->page_size = _page_size;
-}
-
-static inline int tep_is_file_bigendian(struct tep_handle *pevent)
-{
-	return pevent->file_bigendian;
-}
-
-static inline void tep_set_file_bigendian(struct tep_handle *pevent, int endian)
-{
-	pevent->file_bigendian = endian;
-}
-
-static inline int tep_is_host_bigendian(struct tep_handle *pevent)
-{
-	return pevent->host_bigendian;
-}
-
-static inline void tep_set_host_bigendian(struct tep_handle *pevent, int endian)
-{
-	pevent->host_bigendian = endian;
-}
-
-static inline int tep_is_latency_format(struct tep_handle *pevent)
-{
-	return pevent->latency_format;
-}
-
-static inline void tep_set_latency_format(struct tep_handle *pevent, int lat)
-{
-	pevent->latency_format = lat;
-}
+int tep_get_cpus(struct tep_handle *pevent);
+void tep_set_cpus(struct tep_handle *pevent, int cpus);
+int tep_get_long_size(struct tep_handle *pevent);
+void tep_set_long_size(struct tep_handle *pevent, int long_size);
+int tep_get_page_size(struct tep_handle *pevent);
+void tep_set_page_size(struct tep_handle *pevent, int _page_size);
+int tep_is_file_bigendian(struct tep_handle *pevent);
+void tep_set_file_bigendian(struct tep_handle *pevent, int endian);
+int tep_is_host_bigendian(struct tep_handle *pevent);
+void tep_set_host_bigendian(struct tep_handle *pevent, int 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);
 
 struct tep_handle *tep_alloc(void);
 void tep_free(struct tep_handle *pevent);
diff --git a/tools/lib/traceevent/event-plugin.c b/tools/lib/traceevent/event-plugin.c
index f6a9f9363c8d..d15313dbcc8f 100644
--- a/tools/lib/traceevent/event-plugin.c
+++ b/tools/lib/traceevent/event-plugin.c
@@ -28,6 +28,7 @@
 #include <unistd.h>
 #include <dirent.h>
 #include "event-parse.h"
+#include "event-parse-local.h"
 #include "event-utils.h"
 
 #define LOCAL_PLUGIN_DIR ".traceevent/plugins"
diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c
index c4a5404f5f2b..eebbff7cd1a3 100644
--- a/tools/lib/traceevent/parse-filter.c
+++ b/tools/lib/traceevent/parse-filter.c
@@ -25,6 +25,7 @@
 #include <sys/types.h>
 
 #include "event-parse.h"
+#include "event-parse-local.h"
 #include "event-utils.h"
 
 #define COMM "COMM"
diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
index 2ea658268767..dba0bf56a170 100644
--- a/tools/perf/util/trace-event-parse.c
+++ b/tools/perf/util/trace-event-parse.c
@@ -37,10 +37,11 @@ static int get_common_field(struct scripting_context *context,
 	struct tep_format_field *field;
 
 	if (!*size) {
-		if (!pevent->events)
+
+		event = tep_get_events(pevent);
+		if (!event)
 			return 0;
 
-		event = pevent->events[0];
 		field = tep_find_common_field(event, type);
 		if (!field)
 			return 0;
@@ -193,25 +194,29 @@ struct tep_event_format *trace_find_next_event(struct tep_handle *pevent,
 					       struct tep_event_format *event)
 {
 	static int idx;
+	int events_count;
+	struct tep_event_format *all_events;
 
-	if (!pevent || !pevent->events)
+	all_events = tep_get_events(pevent);
+	events_count = tep_get_events_count(pevent);
+	if (!pevent || !all_events || events_count < 1)
 		return NULL;
 
 	if (!event) {
 		idx = 0;
-		return pevent->events[0];
+		return all_events;
 	}
 
-	if (idx < pevent->nr_events && event == pevent->events[idx]) {
+	if (idx < events_count && event == (all_events + idx)) {
 		idx++;
-		if (idx == pevent->nr_events)
+		if (idx == events_count)
 			return NULL;
-		return pevent->events[idx];
+		return (all_events + idx);
 	}
 
-	for (idx = 1; idx < pevent->nr_events; idx++) {
-		if (event == pevent->events[idx - 1])
-			return pevent->events[idx];
+	for (idx = 1; idx < events_count; idx++) {
+		if (event == (all_events + (idx - 1)))
+			return (all_events + idx);
 	}
 	return NULL;
 }
diff --git a/tools/perf/util/trace-event-read.c b/tools/perf/util/trace-event-read.c
index b98ee2a2eb44..cdf6de82a507 100644
--- a/tools/perf/util/trace-event-read.c
+++ b/tools/perf/util/trace-event-read.c
@@ -241,7 +241,7 @@ static int read_header_files(struct tep_handle *pevent)
 		 * The commit field in the page is of type long,
 		 * use that instead, since it represents the kernel.
 		 */
-		tep_set_long_size(pevent, pevent->header_page_size_size);
+		tep_set_long_size(pevent, tep_get_header_page_size(pevent));
 	}
 	free(header_page);
 
-- 
2.17.1

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

end of thread, back to index

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-05 16:22 [PATCH] tools/lib/traceevent, tools/perf: Move struct tep_handler definition in a local header file Steven Rostedt
2018-10-05 16:28 ` Steven Rostedt
2018-10-08 17:32   ` Arnaldo Carvalho de Melo
2018-10-08 19:54     ` Steven Rostedt
2018-10-08 17:31 ` Arnaldo Carvalho de Melo
2018-10-09  5:33 ` [tip:perf/core] tools lib traceevent, perf tools: " tip-bot for Tzvetomir Stoyanov
2019-07-26  3:58 ` [PATCH] tools/lib/traceevent, tools/perf: " Andres Freund
2019-07-26  8:25   ` Tzvetomir Stoyanov
2019-07-26 13:12   ` Steven Rostedt
2019-07-26 20:55     ` Andres Freund
2019-07-26 21:00       ` Steven Rostedt
2019-08-01 14:09       ` Steven Rostedt
  -- strict thread matches above, loose matches on Subject: below --
2018-08-13 15:06 Tzvetomir Stoyanov (VMware)
2018-10-03  0:05 ` Steven Rostedt

Linux-Trace-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-trace-devel/0 linux-trace-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-trace-devel linux-trace-devel/ https://lore.kernel.org/linux-trace-devel \
		linux-trace-devel@vger.kernel.org linux-trace-devel@archiver.kernel.org
	public-inbox-index linux-trace-devel


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-trace-devel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox