diff mbox series

tools/perf,tools/lib/traceevent: Make traceevent APIs more consistent

Message ID 20190322130816.13903-1-tstoyanov@vmware.com (mailing list archive)
State Superseded
Headers show
Series tools/perf,tools/lib/traceevent: Make traceevent APIs more consistent | expand

Commit Message

Tzvetomir Stoyanov March 22, 2019, 1:08 p.m. UTC 
Rename few traceevent APIs, in order to make it more consistent:
tep_pid_is_registered(), tep_file_bigendian(),
tep_is_latency_format(), tep_get_header_page_ts_size(),
tep_set_host_bigendian(), tep_is_host_bigendian() and
tep_data_lat_fmt()

Signed-off-by: Tzvetomir Stoyanov <tstoyanov@vmware.com>
---
 tools/lib/traceevent/event-parse-api.c | 40 +++++++++++++-------------
 tools/lib/traceevent/event-parse.c     | 26 ++++++++---------
 tools/lib/traceevent/event-parse.h     | 18 ++++++------
 tools/lib/traceevent/plugin_kvm.c      |  4 +--
 tools/perf/util/trace-event-read.c     |  2 +-
 tools/perf/util/trace-event.c          |  4 +--
 6 files changed, 47 insertions(+), 47 deletions(-)

Comments

Steven Rostedt March 22, 2019, 2:09 p.m. UTC | #1 
On Fri, 22 Mar 2019 15:08:16 +0200
Tzvetomir Stoyanov <tstoyanov@vmware.com> wrote:

> Rename few traceevent APIs, in order to make it more consistent:
> tep_pid_is_registered(), tep_file_bigendian(),
> tep_is_latency_format(), tep_get_header_page_ts_size(),
> tep_set_host_bigendian(), tep_is_host_bigendian() and
> tep_data_lat_fmt()

Note, change logs should be about "why" not "what". The above is mostly
about "what" although you did state "to make it more consistent" which
is a "why", but we should elaborate more. Something like this:

=====================================
Rename some traceevent APIs for consistency:

 tep_pid_is_registered() to tep_is_pid_registered()

as the convention is "tep_is" not "tep_X_is".

 tep_file_bigendian() to tep_is_file_bigendian()

as boolean operations will now have "tep_is_" notation

 tep_get_header_page_ts_size() to tep_get_header_timestamp_size()

as the latter is more descriptive.

 tep_set_host_bigendian() to tep_set_local_bigendian()
 tep_is_host_bigendian() to tep_is_local_bigendian()

because "host" can be confused with VMs, and "local" is about the local
machine.

 tep_host_bigendian() to tep_is_bigendian()

Again "host" is confusing and we are converting to "tep_is_" notation,
also this one checks the actual machine that is running. For
consistency we have:

  bool tep_is_bigendian(void);
  bool tep_is_file_bigendian(struct tep_handle *tep);
  bool tep_is_local_bigendian(struct tep_handle *tep);

Where tep_is_X_bigendian(tep) returns the saved data in the tep handle,
and tep_is_bigendian() returns the running machine's endianess.

 tep_data_lat_fmt() to tep_data_latency_format()

No need to obfuscate with shorten names here.

Switching all "tep_is_" functions to return bool and not int.
=====================================

That is much more descriptive of the change, and lets anyone that looks
at the git history see exactly why things have changed. With a more
descriptive change log, it will cause less arguments as well.

Please resend with an updated change log.

-- Steve
Steven Rostedt March 22, 2019, 2:34 p.m. UTC | #2 
On Fri, 22 Mar 2019 15:08:16 +0200
Tzvetomir Stoyanov <tstoyanov@vmware.com> wrote:

>  /**
> - * tep_get_header_page_ts_size - get size of the time stamp in the header page
> + * tep_get_header_timestamp_size - get size of the time stamp in the header page

Let's change "time stamp" to "timestamp". Even though it's not quite a
correct English word.


>  
>  /**
> - * tep_file_bigendian - get if the file is in big endian order
> + * tep_is_file_bigendian - get if the file is in big endian order

 tep_is_file_bigendian - return the endian of the file


>   * @pevent: a handle to the tep_handle
>   *
> - * This returns if the file is in big endian order
> - * If @pevent is NULL, 0 is returned.
> + * This returns true if the file is in big endian order
> + * If @pevent is NULL, false is returned.
>   */
> -int tep_file_bigendian(struct tep_handle *pevent)
> +bool tep_is_file_bigendian(struct tep_handle *pevent)
>  {
>  	if (pevent)
> -		return pevent->file_bigendian;
> -	return 0;
> +		return (pevent->file_bigendian == TEP_BIG_ENDIAN);
> +	return false;
>  }
>  
>  /**
> @@ -277,27 +277,27 @@ void tep_set_file_bigendian(struct tep_handle *pevent, enum tep_endian endian)
>  }
>  
>  /**
> - * tep_is_host_bigendian - get if the order of the current host is big endian
> + * tep_is_local_bigendian - get if the local order is big endian


 tep_is_local_bigendian - return the endian of the saved local machine

>   * @pevent: a handle to the tep_handle

 s/@pevent/@tep/

>   *
> - * This gets if the order of the current host is big endian
> + * This gets if the local order is big endian

 This returns true if the saved local machine in @tep is big endian.

>   * If @pevent is NULL, 0 is returned.

 s/@pevent/@tep/

>   */
> -int tep_is_host_bigendian(struct tep_handle *pevent)
> +bool tep_is_local_bigendian(struct tep_handle *pevent)
>  {
>  	if (pevent)
> -		return pevent->host_bigendian;
> +		return (pevent->host_bigendian == TEP_BIG_ENDIAN);
>  	return 0;
>  }
>  
>  /**
> - * tep_set_host_bigendian - set the order of the local host
> + * tep_set_local_bigendian - set the local endian order

 - set the stored local machine endian order

>   * @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
> + * This sets the local endian order

  This sets the endian order for the local machine.


> diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
> index 69af77896283..7d553a72469b 100644
> --- a/tools/lib/traceevent/event-parse.c
> +++ b/tools/lib/traceevent/event-parse.c
> @@ -199,23 +199,23 @@ static const char *find_cmdline(struct tep_handle *pevent, int pid)

>  /**
> - * tep_data_lat_fmt - parse the data for the latency format
> + * tep_data_latency_format - parse the data for the latency format
>   * @pevent: a handle to the pevent

While we modify the functions here, we should also change @pevent to
@tep.

That needs to be stated in the change log too:

  Where modified, the legacy "pevent" name is changed to "tep".

Hmm, actually, lets make this a two patch series. One that removes all
references to "pevent" (in the tools/lib/traceevent/ directory), and
then this one on top of it.

-- Steve


>   * @s: the trace_seq to write to
>   * @record: the record to read from
> @@ -5180,8 +5180,8 @@ static void pretty_print(struct trace_seq *s, void *data, int size, struct tep_e
>   * need rescheduling, in hard/soft interrupt, preempt count
>   * and lock depth) and places it into the trace_seq.
>   */
> -void tep_data_lat_fmt(struct tep_handle *pevent,
> -		      struct trace_seq *s, struct tep_record *record)
> +void tep_data_latency_format(struct tep_handle *pevent,
> +			     struct trace_seq *s, struct tep_record *record)
>  {
>  	static int check_lock_depth = 1;
>  	static int check_migrate_disable = 1;
> @@ -5530,7 +5530,7 @@ void tep_print_event_time(struct tep_handle *pevent, struct trace_seq *s,
>  	}
>  
>  	if (pevent->latency_format) {
> -		tep_data_lat_fmt(pevent, s, record);
> +		tep_data_latency_format(pevent, s, record);
>  	}
>  
>  	if (use_usec_format) {
> @@ -6758,7 +6758,7 @@ struct tep_handle *tep_alloc(void)
>  
>  	if (pevent) {
>  		pevent->ref_count = 1;
> -		pevent->host_bigendian = tep_host_bigendian();
> +		pevent->host_bigendian = tep_is_bigendian();
>  	}
diff mbox series

Patch

diff --git a/tools/lib/traceevent/event-parse-api.c b/tools/lib/traceevent/event-parse-api.c
index 112a83f5caa9..7b17fb1ef30a 100644
--- a/tools/lib/traceevent/event-parse-api.c
+++ b/tools/lib/traceevent/event-parse-api.c
@@ -156,13 +156,13 @@  int tep_get_header_page_size(struct tep_handle *pevent)
 }
 
 /**
- * tep_get_header_page_ts_size - get size of the time stamp in the header page
+ * tep_get_header_timestamp_size - get size of the time stamp in the header page
  * @tep: a handle to the tep_handle
  *
  * This returns size of the time stamp in the header page
  * If @tep is NULL, 0 is returned.
  */
-int tep_get_header_page_ts_size(struct tep_handle *tep)
+int tep_get_header_timestamp_size(struct tep_handle *tep)
 {
 	if (tep)
 		return tep->header_page_ts_size;
@@ -250,17 +250,17 @@  void tep_set_page_size(struct tep_handle *pevent, int _page_size)
 }
 
 /**
- * tep_file_bigendian - get if the file is in big endian order
+ * 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.
+ * This returns true if the file is in big endian order
+ * If @pevent is NULL, false is returned.
  */
-int tep_file_bigendian(struct tep_handle *pevent)
+bool tep_is_file_bigendian(struct tep_handle *pevent)
 {
 	if (pevent)
-		return pevent->file_bigendian;
-	return 0;
+		return (pevent->file_bigendian == TEP_BIG_ENDIAN);
+	return false;
 }
 
 /**
@@ -277,27 +277,27 @@  void tep_set_file_bigendian(struct tep_handle *pevent, enum tep_endian endian)
 }
 
 /**
- * tep_is_host_bigendian - get if the order of the current host is big endian
+ * tep_is_local_bigendian - get if the local order is big endian
  * @pevent: a handle to the tep_handle
  *
- * This gets if the order of the current host is big endian
+ * This gets if the local order is big endian
  * If @pevent is NULL, 0 is returned.
  */
-int tep_is_host_bigendian(struct tep_handle *pevent)
+bool tep_is_local_bigendian(struct tep_handle *pevent)
 {
 	if (pevent)
-		return pevent->host_bigendian;
+		return (pevent->host_bigendian == TEP_BIG_ENDIAN);
 	return 0;
 }
 
 /**
- * tep_set_host_bigendian - set the order of the local host
+ * tep_set_local_bigendian - set the local endian order
  * @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
+ * This sets the local endian order
  */
-void tep_set_host_bigendian(struct tep_handle *pevent, enum tep_endian endian)
+void tep_set_local_bigendian(struct tep_handle *pevent, enum tep_endian endian)
 {
 	if (pevent)
 		pevent->host_bigendian = endian;
@@ -307,14 +307,14 @@  void tep_set_host_bigendian(struct tep_handle *pevent, enum tep_endian 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.
+ * This returns true if the latency output format is configured
+ * If @pevent is NULL, false is returned.
  */
-int tep_is_latency_format(struct tep_handle *pevent)
+bool tep_is_latency_format(struct tep_handle *pevent)
 {
 	if (pevent)
-		return pevent->latency_format;
-	return 0;
+		return !!(pevent->latency_format);
+	return false;
 }
 
 /**
diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 69af77896283..7d553a72469b 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -199,23 +199,23 @@  static const char *find_cmdline(struct tep_handle *pevent, int pid)
 }
 
 /**
- * tep_pid_is_registered - return if a pid has a cmdline registered
+ * tep_is_pid_registered - return if a pid has a cmdline registered
  * @pevent: handle for the pevent
  * @pid: The pid to check if it has a cmdline registered with.
  *
- * Returns 1 if the pid has a cmdline mapped to it
- * 0 otherwise.
+ * Returns true if the pid has a cmdline mapped to it
+ * false otherwise.
  */
-int tep_pid_is_registered(struct tep_handle *pevent, int pid)
+bool tep_is_pid_registered(struct tep_handle *pevent, int pid)
 {
 	const struct tep_cmdline *comm;
 	struct tep_cmdline key;
 
 	if (!pid)
-		return 1;
+		return true;
 
 	if (!pevent->cmdlines && cmdline_init(pevent))
-		return 0;
+		return false;
 
 	key.pid = pid;
 
@@ -223,8 +223,8 @@  int tep_pid_is_registered(struct tep_handle *pevent, int pid)
 		       sizeof(*pevent->cmdlines), cmdline_cmp);
 
 	if (comm)
-		return 1;
-	return 0;
+		return true;
+	return false;
 }
 
 /*
@@ -5171,7 +5171,7 @@  static void pretty_print(struct trace_seq *s, void *data, int size, struct tep_e
 }
 
 /**
- * tep_data_lat_fmt - parse the data for the latency format
+ * tep_data_latency_format - parse the data for the latency format
  * @pevent: a handle to the pevent
  * @s: the trace_seq to write to
  * @record: the record to read from
@@ -5180,8 +5180,8 @@  static void pretty_print(struct trace_seq *s, void *data, int size, struct tep_e
  * need rescheduling, in hard/soft interrupt, preempt count
  * and lock depth) and places it into the trace_seq.
  */
-void tep_data_lat_fmt(struct tep_handle *pevent,
-		      struct trace_seq *s, struct tep_record *record)
+void tep_data_latency_format(struct tep_handle *pevent,
+			     struct trace_seq *s, struct tep_record *record)
 {
 	static int check_lock_depth = 1;
 	static int check_migrate_disable = 1;
@@ -5530,7 +5530,7 @@  void tep_print_event_time(struct tep_handle *pevent, struct trace_seq *s,
 	}
 
 	if (pevent->latency_format) {
-		tep_data_lat_fmt(pevent, s, record);
+		tep_data_latency_format(pevent, s, record);
 	}
 
 	if (use_usec_format) {
@@ -6758,7 +6758,7 @@  struct tep_handle *tep_alloc(void)
 
 	if (pevent) {
 		pevent->ref_count = 1;
-		pevent->host_bigendian = tep_host_bigendian();
+		pevent->host_bigendian = tep_is_bigendian();
 	}
 
 	return pevent;
diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index 4b64658334de..960aef7340e4 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -412,7 +412,7 @@  void tep_set_flag(struct tep_handle *tep, int flag);
 void tep_clear_flag(struct tep_handle *tep, enum tep_flag flag);
 bool tep_check_flags(struct tep_handle *tep, enum tep_flag flags);
 
-static inline int tep_host_bigendian(void)
+static inline int tep_is_bigendian(void)
 {
 	unsigned char str[] = { 0x1, 0x2, 0x3, 0x4 };
 	unsigned int val;
@@ -440,7 +440,7 @@  int tep_register_function(struct tep_handle *pevent, char *name,
 			  unsigned long long addr, char *mod);
 int tep_register_print_string(struct tep_handle *pevent, const char *fmt,
 			      unsigned long long addr);
-int tep_pid_is_registered(struct tep_handle *pevent, int pid);
+bool tep_is_pid_registered(struct tep_handle *pevent, int pid);
 
 void tep_print_event_task(struct tep_handle *pevent, struct trace_seq *s,
 			  struct tep_event *event,
@@ -525,8 +525,8 @@  tep_find_event_by_name(struct tep_handle *pevent, const char *sys, const char *n
 struct tep_event *
 tep_find_event_by_record(struct tep_handle *pevent, struct tep_record *record);
 
-void tep_data_lat_fmt(struct tep_handle *pevent,
-		      struct trace_seq *s, struct tep_record *record);
+void tep_data_latency_format(struct tep_handle *pevent,
+			     struct trace_seq *s, struct tep_record *record);
 int tep_data_type(struct tep_handle *pevent, struct tep_record *rec);
 int tep_data_pid(struct tep_handle *pevent, struct tep_record *rec);
 int tep_data_preempt_count(struct tep_handle *pevent, struct tep_record *rec);
@@ -560,16 +560,16 @@  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_file_bigendian(struct tep_handle *pevent);
+bool 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);
+bool tep_is_local_bigendian(struct tep_handle *pevent);
+void tep_set_local_bigendian(struct tep_handle *pevent, enum tep_endian endian);
+bool tep_is_latency_format(struct tep_handle *pevent);
 void tep_set_latency_format(struct tep_handle *pevent, int lat);
 int tep_get_header_page_size(struct tep_handle *pevent);
 void tep_set_parsing_failures(struct tep_handle *tep, int parsing_failures);
 int tep_get_parsing_failures(struct tep_handle *tep);
-int tep_get_header_page_ts_size(struct tep_handle *tep);
+int tep_get_header_timestamp_size(struct tep_handle *tep);
 bool tep_is_old_format(struct tep_handle *pevent);
 void tep_set_print_raw(struct tep_handle *tep, int print_raw);
 void tep_set_test_filters(struct tep_handle *tep, int test_filters);
diff --git a/tools/lib/traceevent/plugin_kvm.c b/tools/lib/traceevent/plugin_kvm.c
index 64b9c25a1fd3..688e5d97d7a7 100644
--- a/tools/lib/traceevent/plugin_kvm.c
+++ b/tools/lib/traceevent/plugin_kvm.c
@@ -389,8 +389,8 @@  static int kvm_mmu_print_role(struct trace_seq *s, struct tep_record *record,
 	 * We can only use the structure if file is of the same
 	 * endianness.
 	 */
-	if (tep_file_bigendian(event->pevent) ==
-	    tep_is_host_bigendian(event->pevent)) {
+	if (tep_is_file_bigendian(event->pevent) ==
+	    tep_is_local_bigendian(event->pevent)) {
 
 		trace_seq_printf(s, "%u q%u%s %s%s %spae %snxe %swp%s%s%s",
 				 role.level,
diff --git a/tools/perf/util/trace-event-read.c b/tools/perf/util/trace-event-read.c
index efe2f58cff4e..48d53d8e3e16 100644
--- a/tools/perf/util/trace-event-read.c
+++ b/tools/perf/util/trace-event-read.c
@@ -442,7 +442,7 @@  ssize_t trace_report(int fd, struct trace_event *tevent, bool __repipe)
 
 	tep_set_flag(pevent, TEP_NSEC_OUTPUT);
 	tep_set_file_bigendian(pevent, file_bigendian);
-	tep_set_host_bigendian(pevent, host_bigendian);
+	tep_set_local_bigendian(pevent, host_bigendian);
 
 	if (do_read(buf, 1) < 0)
 		goto out;
diff --git a/tools/perf/util/trace-event.c b/tools/perf/util/trace-event.c
index cbe0dd758e3a..01b9d89bf5bf 100644
--- a/tools/perf/util/trace-event.c
+++ b/tools/perf/util/trace-event.c
@@ -40,7 +40,7 @@  int trace_event__init(struct trace_event *t)
 
 static int trace_event__init2(void)
 {
-	int be = tep_host_bigendian();
+	int be = tep_is_bigendian();
 	struct tep_handle *pevent;
 
 	if (trace_event__init(&tevent))
@@ -49,7 +49,7 @@  static int trace_event__init2(void)
 	pevent = tevent.pevent;
 	tep_set_flag(pevent, TEP_NSEC_OUTPUT);
 	tep_set_file_bigendian(pevent, be);
-	tep_set_host_bigendian(pevent, be);
+	tep_set_local_bigendian(pevent, be);
 	tevent_initialized = true;
 	return 0;
 }