Skip to contentSkip to conversation

tools lib traceevent: Pulling in updates from trace-cmd

PATCH In review Steven Rostedt 3 participants 31 messages Mar 24, 2015 Stable branch

lore
Share & tools
lore
#0 lore raw

cover tools lib traceevent: Pulling in updates from trace-cmd

Arnaldo,

I took a look at the differences between what I have in trace-cmd and what is sitting in tools/lib/traceevent, and I put together a patch set that brings in fixes and updates to libtraceevent.

-- Steve

Local SHA1: bd9d9602719a24d0843bb04e52b6bf98d9e90881

Josef Bacik (1):
tools lib traceevent: Handle NULL comm name

Steven Rostedt (1):
tools lib traceevent: Make plugin options either string or boolean

Steven Rostedt (Red Hat) (7):
tools lib traceevent: Copy trace_clock and free it
tools lib traceevent: Handle '%z' in bprint format
tools lib traceevent: Add pevent_data_pid_from_comm()
tools lib traceevent: Fix whitespace error
tools lib traceevent kbuffer: Remove extra update to data pointer in PADDING tools lib traceevent: Add way to find sub buffer boundary
tools lib traceevent: Free filter tokens in process_filter()

----

 tools/lib/traceevent/event-parse.c   | 109 ++++++++++++++++++++++++++++++++++-
 tools/lib/traceevent/event-parse.h   |  13 ++++-
 tools/lib/traceevent/event-plugin.c  |  60 +++++++++++++++++--
 tools/lib/traceevent/kbuffer-parse.c |  12 +++-
 tools/lib/traceevent/kbuffer.h       |   1 +
 tools/lib/traceevent/parse-filter.c  |   2 +
 6 files changed, 185 insertions(+), 12 deletions(-)
#1 lore raw

PATCH 1/9 tools lib traceevent: Handle NULL comm name

From: Josef Bacik <[email protected]>

It is possible that a pid has no associated comm attached to it, although it can still be passed to pevent_register_comm(). But if comm is NULL, it will cause strdup() to segfault. To prevent this from happening, if comm is NULL use the default "<...>" name for the pid.

---
 tools/lib/traceevent/event-parse.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
tools/lib/traceevent/event-parse.c +4 -1
index d7c37a7d9255..fdcb89be5e67 100644
@@ -304,7 +304,10 @@ int pevent_register_comm(struct pevent *pevent, const char *comm, int pid)
 	if (!item)
 		return -1;
 
-	item->comm = strdup(comm);
+	if (comm)
+		item->comm = strdup(comm);
+	else
+		item->comm = strdup("<...>");
 	if (!item->comm) {
 		free(item);
 		return -1;
Signature · 1 line
2.1.4
#2 lore raw

[tip:perf/core] tools lib traceevent: Handle NULL comm name

tools lib traceevent: Handle NULL comm name
Commit-ID:  deab6f55a2fe6fe044af61c9aad6a8e90cda6499
Gitweb:     http://git.kernel.org/tip/deab6f55a2fe6fe044af61c9aad6a8e90cda6499
Author:     Josef Bacik <[email protected]>
AuthorDate: Tue, 24 Mar 2015 09:57:49 -0400
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 24 Mar 2015 12:10:26 -0300

tools lib traceevent: Handle NULL comm name

It is possible that a pid has no associated comm attached to it, although it can still be passed to pevent_register_comm().

But if comm is NULL, it will cause strdup() to segfault. To prevent this from happening, if comm is NULL use the default "<...>" name for the pid.

---
 tools/lib/traceevent/event-parse.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
tools/lib/traceevent/event-parse.c +4 -1
index 8e5e4f6..31d4e7d 100644
@@ -304,7 +304,10 @@ int pevent_register_comm(struct pevent *pevent, const char *comm, int pid)
 	if (!item)
 		return -1;
 
-	item->comm = strdup(comm);
+	if (comm)
+		item->comm = strdup(comm);
+	else
+		item->comm = strdup("<...>");
 	if (!item->comm) {
 		free(item);
 		return -1;
#3 lore raw

PATCH 2/9 tools lib traceevent: Copy trace_clock and free it

In reply to Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

The pevent->trace_clock should not be a direct pointer to what was given. It should be copied and freed.

Note, valgrind pointed this out when a caller passed in a pointer that needed to be freed and it never was. Ideally, pevent should copy it (which this change does), and free the copy. It's up to the caller to free the clock string passed in.

---
 tools/lib/traceevent/event-parse.c | 10 ++++++++--
 tools/lib/traceevent/event-parse.h |  2 +-
 2 files changed, 9 insertions(+), 3 deletions(-)
tools/lib/traceevent/event-parse.c +8 -2
index fdcb89be5e67..9709e202a7e5 100644
@@ -321,9 +321,14 @@ int pevent_register_comm(struct pevent *pevent, const char *comm, int pid)
 	return 0;
 }
 
-void pevent_register_trace_clock(struct pevent *pevent, char *trace_clock)
+int pevent_register_trace_clock(struct pevent *pevent, const char *trace_clock)
 {
-	pevent->trace_clock = trace_clock;
+	pevent->trace_clock = strdup(trace_clock);
+	if (!pevent->trace_clock) {
+		errno = ENOMEM;
+		return -1;
+	}
+	return 0;
 }
 
 struct func_map {
@@ -6349,6 +6354,7 @@ void pevent_free(struct pevent *pevent)
 		free_handler(handle);
 	}
 
+	free(pevent->trace_clock);
 	free(pevent->events);
 	free(pevent->sort_events);
 
tools/lib/traceevent/event-parse.h +1 -1
index 6abda54d76f2..84e554f84574 100644
@@ -599,7 +599,7 @@ enum trace_flag_type {
 };
 
 int pevent_register_comm(struct pevent *pevent, const char *comm, int pid);
-void pevent_register_trace_clock(struct pevent *pevent, char *trace_clock);
+int pevent_register_trace_clock(struct pevent *pevent, const char *trace_clock);
 int pevent_register_function(struct pevent *pevent, char *name,
 			     unsigned long long addr, char *mod);
 int pevent_register_print_string(struct pevent *pevent, const char *fmt,
Signature · 1 line
2.1.4
#4 lore raw

Re: [PATCH 2/9] tools lib traceevent: Copy trace_clock and free it

Em Tue, Mar 24, 2015 at 09:57:50AM -0400, Steven Rostedt escreveu:

Quoted 13 lines

From: "Steven Rostedt (Red Hat)" <[email protected]>

The pevent->trace_clock should not be a direct pointer to what was given. It should be copied and freed.

Note, valgrind pointed this out when a caller passed in a pointer that needed to be freed and it never was. Ideally, pevent should copy it (which this change does), and free the copy. It's up to the caller to free the clock string passed in.

+++ b/tools/lib/traceevent/event-parse.c

tools/lib/traceevent/event-parse.c +0 -0
@@ -321,9 +321,14 @@ int pevent_register_comm(struct pevent *pevent, const char *comm, int pid)
 	return 0;

tools/lib/traceevent/event-parse.c int pevent_register_comm(struct pevent *pevent, const char *comm, int pid) · line 329

Quoted 10 lines
tools/lib/traceevent/event-parse.c +5 -2
 }
 
-void pevent_register_trace_clock(struct pevent *pevent, char *trace_clock)
+int pevent_register_trace_clock(struct pevent *pevent, const char *trace_clock)
 {
-	pevent->trace_clock = trace_clock;
+	pevent->trace_clock = strdup(trace_clock);
+	if (!pevent->trace_clock) {
+		errno = ENOMEM;
+		return -1;

Humm, strdup actually sets errno already, from its man page:

-------------------

RETURN VALUE
On success, the strdup() function returns a pointer to the
duplicated string. It returns NULL if insufficient memory was available, with errno set to indicate the cause of the error.

-------------------

Applying anyway, as this doesn't introduces a problem, right?

- ARnaldo

#5 lore raw

Re: [PATCH 2/9] tools lib traceevent: Copy trace_clock and free it

Quoted 9 lines · On Tue, 24 Mar 2015 12:14:49 -0300 Arnaldo Carvalho de Melo <[email protected]>…

Humm, strdup actually sets errno already, from its man page:

-------------------

RETURN VALUE
On success, the strdup() function returns a pointer to the
duplicated string. It returns NULL if insufficient memory was available, with errno set to indicate the cause of the error.

Quoted 5 lines

-------------------

Applying anyway, as this doesn't introduces a problem, right?

Ah, I didn't realize that. I probably should of. The original code just had a "die()" in it. But I know how much Namhyung loves those ;-)

I removed the die() and noticed that the other removed "die()"s also set errno = ENOMEM. Those probably could also be removed as they happen after "remalloc" and such.

-- Steve

#6 lore raw

[tip:perf/core] tools lib traceevent: Copy trace_clock and free it

In reply to Steven Rostedt
tools lib traceevent: Copy trace_clock and free it
Commit-ID:  99ad1417db2e83530b88e3eff1a40285d6ebfee9
Gitweb:     http://git.kernel.org/tip/99ad1417db2e83530b88e3eff1a40285d6ebfee9
Author:     Steven Rostedt (Red Hat) <[email protected]>
AuthorDate: Tue, 24 Mar 2015 09:57:50 -0400
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 24 Mar 2015 12:11:19 -0300

tools lib traceevent: Copy trace_clock and free it

The pevent->trace_clock should not be a direct pointer to what was given. It should be copied and freed.

Note, valgrind pointed this out when a caller passed in a pointer that needed to be freed and it never was. Ideally, pevent should copy it (which this change does), and free the copy. It's up to the caller to free the clock string passed in.

---
 tools/lib/traceevent/event-parse.c | 10 ++++++++--
 tools/lib/traceevent/event-parse.h |  2 +-
 2 files changed, 9 insertions(+), 3 deletions(-)
tools/lib/traceevent/event-parse.c +8 -2
index 31d4e7d..e71c472 100644
@@ -321,9 +321,14 @@ int pevent_register_comm(struct pevent *pevent, const char *comm, int pid)
 	return 0;
 }
 
-void pevent_register_trace_clock(struct pevent *pevent, char *trace_clock)
+int pevent_register_trace_clock(struct pevent *pevent, const char *trace_clock)
 {
-	pevent->trace_clock = trace_clock;
+	pevent->trace_clock = strdup(trace_clock);
+	if (!pevent->trace_clock) {
+		errno = ENOMEM;
+		return -1;
+	}
+	return 0;
 }
 
 struct func_map {
@@ -6352,6 +6357,7 @@ void pevent_free(struct pevent *pevent)
 		free_handler(handle);
 	}
 
+	free(pevent->trace_clock);
 	free(pevent->events);
 	free(pevent->sort_events);
 
tools/lib/traceevent/event-parse.h +1 -1
index 6abda54..84e554f 100644
@@ -599,7 +599,7 @@ enum trace_flag_type {
 };
 
 int pevent_register_comm(struct pevent *pevent, const char *comm, int pid);
-void pevent_register_trace_clock(struct pevent *pevent, char *trace_clock);
+int pevent_register_trace_clock(struct pevent *pevent, const char *trace_clock);
 int pevent_register_function(struct pevent *pevent, char *name,
 			     unsigned long long addr, char *mod);
 int pevent_register_print_string(struct pevent *pevent, const char *fmt,
#7 lore raw

PATCH 3/9 tools lib traceevent: Handle %z in bprint format

In reply to Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

The %z printf specifier was not handled making trace_printk()s in the kernel that used this break on output.

---
 tools/lib/traceevent/event-parse.c | 4 ++++
 1 file changed, 4 insertions(+)
tools/lib/traceevent/event-parse.c +4 -0
index 9709e202a7e5..5795d9451063 100644
@@ -4005,6 +4005,10 @@ static struct print_arg *make_bprint_args(char *fmt, void *data, int size, struc
 				goto process_again;
 			case '.':
 				goto process_again;
+			case 'z':
+			case 'Z':
+				ls = 1;
+				goto process_again;
 			case 'p':
 				ls = 1;
 				/* fall through */
Signature · 1 line
2.1.4
#8 lore raw

[tip:perf/core] tools lib traceevent: Handle %z in bprint format

tools lib traceevent: Handle %z in bprint format
Commit-ID:  55426296963c678650e26fc5e61ea75a5598ef5a
Gitweb:     http://git.kernel.org/tip/55426296963c678650e26fc5e61ea75a5598ef5a
Author:     Steven Rostedt (Red Hat) <[email protected]>
AuthorDate: Tue, 24 Mar 2015 09:57:51 -0400
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 24 Mar 2015 12:15:12 -0300

tools lib traceevent: Handle %z in bprint format

The %z printf specifier was not handled making trace_printk()s in the kernel that used this break on output.

---
 tools/lib/traceevent/event-parse.c | 4 ++++
 1 file changed, 4 insertions(+)
tools/lib/traceevent/event-parse.c +4 -0
index e71c472..bdb1dd6 100644
@@ -4008,6 +4008,10 @@ static struct print_arg *make_bprint_args(char *fmt, void *data, int size, struc
 				goto process_again;
 			case '.':
 				goto process_again;
+			case 'z':
+			case 'Z':
+				ls = 1;
+				goto process_again;
 			case 'p':
 				ls = 1;
 				/* fall through */
#9 lore raw

PATCH 4/9 tools lib traceevent: Add pevent_data_pid_from_comm()

In reply to Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

There is a pevent_data_comm_from_pid() that returns the cmdline stored for a given pid in order for users to map pids to comms, but there's no method to convert a comm back to a pid. This is useful for filters that specify a comm instead of a PID (it's faster than searching each individual event).

Add a way to retrieve a comm from a pid. Since there can be more than one pid associated to a comm, it returns a data structure that lets the user iterate over all the saved comms for a given pid.

---
 tools/lib/traceevent/event-parse.c | 90 ++++++++++++++++++++++++++++++++++++++
 tools/lib/traceevent/event-parse.h |  5 +++
 2 files changed, 95 insertions(+)
tools/lib/traceevent/event-parse.c +90 -0
index 5795d9451063..8cc3e894ff46 100644
@@ -4951,6 +4951,96 @@ const char *pevent_data_comm_from_pid(struct pevent *pevent, int pid)
 	return comm;
 }
 
+static struct cmdline *
+pid_from_cmdlist(struct pevent *pevent, const char *comm, struct cmdline *next)
+{
+	struct cmdline_list *cmdlist = (struct cmdline_list *)next;
+
+	if (cmdlist)
+		cmdlist = cmdlist->next;
+	else
+		cmdlist = pevent->cmdlist;
+
+	while (cmdlist && strcmp(cmdlist->comm, comm) != 0)
+		cmdlist = cmdlist->next;
+
+	return (struct cmdline *)cmdlist;
+}
+
+/**
+ * pevent_data_pid_from_comm - return the pid from a given comm
+ * @pevent: a handle to the pevent
+ * @comm: the cmdline to find the pid from
+ * @next: the cmdline structure to find the next comm
+ *
+ * This returns the cmdline structure that holds a pid for a given
+ * comm, or NULL if none found. As there may be more than one pid for
+ * a given comm, the result of this call can be passed back into
+ * a recurring call in the @next paramater, and then it will find the
+ * next pid.
+ * Also, it does a linear seach, so it may be slow.
+ */
+struct cmdline *pevent_data_pid_from_comm(struct pevent *pevent, const char *comm,
+					  struct cmdline *next)
+{
+	struct cmdline *cmdline;
+
+	/*
+	 * If the cmdlines have not been converted yet, then use
+	 * the list.
+	 */
+	if (!pevent->cmdlines)
+		return pid_from_cmdlist(pevent, comm, next);
+
+	if (next) {
+		/*
+		 * The next pointer could have been still from
+		 * a previous call before cmdlines were created
+		 */
+		if (next < pevent->cmdlines ||
+		    next >= pevent->cmdlines + pevent->cmdline_count)
+			next = NULL;
+		else
+			cmdline  = next++;
+	}
+
+	if (!next)
+		cmdline = pevent->cmdlines;
+
+	while (cmdline < pevent->cmdlines + pevent->cmdline_count) {
+		if (strcmp(cmdline->comm, comm) == 0)
+			return cmdline;
+		cmdline++;
+	}
+	return NULL;
+}
+
+/**
+ * pevent_cmdline_pid - return the pid associated to a given cmdline
+ * @cmdline: The cmdline structure to get the pid from
+ *
+ * Returns the pid for a give cmdline. If @cmdline is NULL, then
+ * -1 is returned.
+ */
+int pevent_cmdline_pid(struct pevent *pevent, struct cmdline *cmdline)
+{
+	struct cmdline_list *cmdlist = (struct cmdline_list *)cmdline;
+
+	if (!cmdline)
+		return -1;
+
+	/*
+	 * If cmdlines have not been created yet, or cmdline is
+	 * not part of the array, then treat it as a cmdlist instead.
+	 */
+	if (!pevent->cmdlines || 
+	    cmdline < pevent->cmdlines ||
+	    cmdline >= pevent->cmdlines + pevent->cmdline_count)
+		return cmdlist->pid;
+
+	return cmdline->pid;
+}
+
 /**
  * pevent_data_comm_from_pid - parse the data into the print format
  * @s: the trace_seq to write to
tools/lib/traceevent/event-parse.h +5 -0
index 84e554f84574..8bd7c6a4cbd7 100644
@@ -678,6 +678,11 @@ int pevent_data_type(struct pevent *pevent, struct pevent_record *rec);
 struct event_format *pevent_data_event_from_type(struct pevent *pevent, int type);
 int pevent_data_pid(struct pevent *pevent, struct pevent_record *rec);
 const char *pevent_data_comm_from_pid(struct pevent *pevent, int pid);
+struct cmdline;
+struct cmdline *pevent_data_pid_from_comm(struct pevent *pevent, const char *comm,
+					  struct cmdline *next);
+int pevent_cmdline_pid(struct pevent *pevent, struct cmdline *cmdline);
+
 void pevent_event_info(struct trace_seq *s, struct event_format *event,
 		       struct pevent_record *record);
 int pevent_strerror(struct pevent *pevent, enum pevent_errno errnum,
Signature · 1 line
2.1.4
#10 lore raw

Re: [PATCH 4/9] tools lib traceevent: Add pevent_data_pid_from_comm()

Em Tue, Mar 24, 2015 at 09:57:52AM -0400, Steven Rostedt escreveu:

Quoted 22 lines

From: "Steven Rostedt (Red Hat)" <[email protected]>

There is a pevent_data_comm_from_pid() that returns the cmdline stored for a given pid in order for users to map pids to comms, but there's no method to convert a comm back to a pid. This is useful for filters that specify a comm instead of a PID (it's faster than searching each individual event).

Add a way to retrieve a comm from a pid. Since there can be more than one pid associated to a comm, it returns a data structure that lets the user iterate over all the saved comms for a given pid.

---
 tools/lib/traceevent/event-parse.c | 90 ++++++++++++++++++++++++++++++++++++++
 tools/lib/traceevent/event-parse.h |  5 +++
 2 files changed, 95 insertions(+)
tools/lib/traceevent/event-parse.c +0 -0
index 5795d9451063..8cc3e894ff46 100644

tools/lib/traceevent/event-parse.c const char *pevent_data_comm_from_pid(struct pevent *pevent, int pid) · line 4953

Quoted 3 lines
tools/lib/traceevent/event-parse.c +0 -0
@@ -4951,6 +4951,96 @@ const char *pevent_data_comm_from_pid(struct pevent *pevent, int pid)
 	return comm;
 }
 

I am fixing this up:

[acme@ssdandy linux]$ am /wb/1.patch && make -C tools/perf LIBBABELTRACE_DIR=/opt/libbabeltrace/ O=/tmp/build/perf install-bin
Applying: tools lib traceevent: Add pevent_data_pid_from_comm() /home/acme/git/linux/.git/rebase-apply/patch:96: trailing whitespace.

	if (!pevent->cmdlines || 

warning: 1 line adds whitespace errors. tools/lib/traceevent/event-parse.c:5039: trailing whitespace.
+ if (!pevent->cmdlines ||
[acme@ssdandy linux]$

#11 lore raw

[tip:perf/core] tools lib traceevent: Add pevent_data_pid_from_comm()

In reply to Steven Rostedt
tools lib traceevent: Add pevent_data_pid_from_comm()
Commit-ID:  2771984c7f5e6cab812e86ec152da4fb5f6df908
Gitweb:     http://git.kernel.org/tip/2771984c7f5e6cab812e86ec152da4fb5f6df908
Author:     Steven Rostedt (Red Hat) <[email protected]>
AuthorDate: Tue, 24 Mar 2015 09:57:52 -0400
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 24 Mar 2015 12:19:06 -0300

tools lib traceevent: Add pevent_data_pid_from_comm()

There is a pevent_data_comm_from_pid() that returns the cmdline stored for a given pid in order for users to map pids to comms, but there's no method to convert a comm back to a pid. This is useful for filters that specify a comm instead of a PID (it's faster than searching each individual event).

Add a way to retrieve a comm from a pid. Since there can be more than one pid associated to a comm, it returns a data structure that lets the user iterate over all the saved comms for a given pid.

---
 tools/lib/traceevent/event-parse.c | 90 ++++++++++++++++++++++++++++++++++++++
 tools/lib/traceevent/event-parse.h |  5 +++
 2 files changed, 95 insertions(+)
tools/lib/traceevent/event-parse.c +90 -0
index bdb1dd6..bc22722 100644
@@ -4954,6 +4954,96 @@ const char *pevent_data_comm_from_pid(struct pevent *pevent, int pid)
 	return comm;
 }
 
+static struct cmdline *
+pid_from_cmdlist(struct pevent *pevent, const char *comm, struct cmdline *next)
+{
+	struct cmdline_list *cmdlist = (struct cmdline_list *)next;
+
+	if (cmdlist)
+		cmdlist = cmdlist->next;
+	else
+		cmdlist = pevent->cmdlist;
+
+	while (cmdlist && strcmp(cmdlist->comm, comm) != 0)
+		cmdlist = cmdlist->next;
+
+	return (struct cmdline *)cmdlist;
+}
+
+/**
+ * pevent_data_pid_from_comm - return the pid from a given comm
+ * @pevent: a handle to the pevent
+ * @comm: the cmdline to find the pid from
+ * @next: the cmdline structure to find the next comm
+ *
+ * This returns the cmdline structure that holds a pid for a given
+ * comm, or NULL if none found. As there may be more than one pid for
+ * a given comm, the result of this call can be passed back into
+ * a recurring call in the @next paramater, and then it will find the
+ * next pid.
+ * Also, it does a linear seach, so it may be slow.
+ */
+struct cmdline *pevent_data_pid_from_comm(struct pevent *pevent, const char *comm,
+					  struct cmdline *next)
+{
+	struct cmdline *cmdline;
+
+	/*
+	 * If the cmdlines have not been converted yet, then use
+	 * the list.
+	 */
+	if (!pevent->cmdlines)
+		return pid_from_cmdlist(pevent, comm, next);
+
+	if (next) {
+		/*
+		 * The next pointer could have been still from
+		 * a previous call before cmdlines were created
+		 */
+		if (next < pevent->cmdlines ||
+		    next >= pevent->cmdlines + pevent->cmdline_count)
+			next = NULL;
+		else
+			cmdline  = next++;
+	}
+
+	if (!next)
+		cmdline = pevent->cmdlines;
+
+	while (cmdline < pevent->cmdlines + pevent->cmdline_count) {
+		if (strcmp(cmdline->comm, comm) == 0)
+			return cmdline;
+		cmdline++;
+	}
+	return NULL;
+}
+
+/**
+ * pevent_cmdline_pid - return the pid associated to a given cmdline
+ * @cmdline: The cmdline structure to get the pid from
+ *
+ * Returns the pid for a give cmdline. If @cmdline is NULL, then
+ * -1 is returned.
+ */
+int pevent_cmdline_pid(struct pevent *pevent, struct cmdline *cmdline)
+{
+	struct cmdline_list *cmdlist = (struct cmdline_list *)cmdline;
+
+	if (!cmdline)
+		return -1;
+
+	/*
+	 * If cmdlines have not been created yet, or cmdline is
+	 * not part of the array, then treat it as a cmdlist instead.
+	 */
+	if (!pevent->cmdlines ||
+	    cmdline < pevent->cmdlines ||
+	    cmdline >= pevent->cmdlines + pevent->cmdline_count)
+		return cmdlist->pid;
+
+	return cmdline->pid;
+}
+
 /**
  * pevent_data_comm_from_pid - parse the data into the print format
  * @s: the trace_seq to write to
tools/lib/traceevent/event-parse.h +5 -0
index 84e554f..8bd7c6a 100644
@@ -678,6 +678,11 @@ int pevent_data_type(struct pevent *pevent, struct pevent_record *rec);
 struct event_format *pevent_data_event_from_type(struct pevent *pevent, int type);
 int pevent_data_pid(struct pevent *pevent, struct pevent_record *rec);
 const char *pevent_data_comm_from_pid(struct pevent *pevent, int pid);
+struct cmdline;
+struct cmdline *pevent_data_pid_from_comm(struct pevent *pevent, const char *comm,
+					  struct cmdline *next);
+int pevent_cmdline_pid(struct pevent *pevent, struct cmdline *cmdline);
+
 void pevent_event_info(struct trace_seq *s, struct event_format *event,
 		       struct pevent_record *record);
 int pevent_strerror(struct pevent *pevent, enum pevent_errno errnum,
#12 lore raw

PATCH 5/9 tools lib traceevent: Fix whitespace error

In reply to Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

---
 tools/lib/traceevent/event-parse.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/lib/traceevent/event-parse.c +1 -1
index 8cc3e894ff46..aff743710001 100644
@@ -5033,7 +5033,7 @@ int pevent_cmdline_pid(struct pevent *pevent, struct cmdline *cmdline)
 	 * If cmdlines have not been created yet, or cmdline is
 	 * not part of the array, then treat it as a cmdlist instead.
 	 */
-	if (!pevent->cmdlines || 
+	if (!pevent->cmdlines ||
 	    cmdline < pevent->cmdlines ||
 	    cmdline >= pevent->cmdlines + pevent->cmdline_count)
 		return cmdlist->pid;
Signature · 1 line
2.1.4
#13 lore raw

Re: [PATCH 5/9] tools lib traceevent: Fix whitespace error

Em Tue, Mar 24, 2015 at 09:57:53AM -0400, Steven Rostedt escreveu:

Quoted 1 line

From: "Steven Rostedt (Red Hat)" <[email protected]>

Ok, you caught me on this one, squashed with the previous one :-)

- Arnaldo

Quoted 21 lines
---
 tools/lib/traceevent/event-parse.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/lib/traceevent/event-parse.c +1 -1
index 8cc3e894ff46..aff743710001 100644
@@ -5033,7 +5033,7 @@ int pevent_cmdline_pid(struct pevent *pevent, struct cmdline *cmdline)
 	 * If cmdlines have not been created yet, or cmdline is
 	 * not part of the array, then treat it as a cmdlist instead.
 	 */
-	if (!pevent->cmdlines || 
+	if (!pevent->cmdlines ||
 	    cmdline < pevent->cmdlines ||
 	    cmdline >= pevent->cmdlines + pevent->cmdline_count)
 		return cmdlist->pid;
Signature · 1 line
2.1.4
#14 lore raw

Re: [PATCH 5/9] tools lib traceevent: Fix whitespace error

Quoted 4 lines · On Tue, 24 Mar 2015 12:19:04 -0300 Arnaldo Carvalho de Melo <[email protected]>…

Em Tue, Mar 24, 2015 at 09:57:53AM -0400, Steven Rostedt escreveu:

Quoted 1 line

From: "Steven Rostedt (Red Hat)" <[email protected]>

Ok, you caught me on this one, squashed with the previous one :-)

Heh, if I noticed that this fixed a previous patch in the series, I would have squashed it myself :-)

-- Steve

#15 lore raw

PATCH 6/9 tools lib traceevent: Make plugin options either string or boolean

In reply to Steven Rostedt

From: Steven Rostedt <[email protected]>

When a plugin option is defined, by default it is a boolean (true or false). If the option is something else, then it needs to set its "value" field to a default string other than NULL (can be just ""). If the value is not set then the option is considered boolean, and the updating of the option value will be handled accordingly.

---
 tools/lib/traceevent/event-parse.h  |  6 +++-
 tools/lib/traceevent/event-plugin.c | 60 +++++++++++++++++++++++++++++++++----
 2 files changed, 59 insertions(+), 7 deletions(-)
tools/lib/traceevent/event-parse.h +5 -1
index 8bd7c6a4cbd7..8ca1b8ee50da 100644
@@ -116,7 +116,7 @@ struct pevent_plugin_option {
 	char				*name;
 	char				*plugin_alias;
 	char				*description;
-	char				*value;
+	const char			*value;
 	void				*priv;
 	int				set;
 };
@@ -154,6 +154,10 @@ struct pevent_plugin_option {
  *   .plugin_alias is used to give a shorter name to access
  *   the vairable. Useful if a plugin handles more than one event.
  *
+ *   If .value is not set, then it is considered a boolean and only
+ *   .set will be processed. If .value is defined, then it is considered
+ *   a string option and .set will be ignored.
+ *
  * PEVENT_PLUGIN_ALIAS: (optional)
  *   The name to use for finding options (uses filename if not defined)
  */
tools/lib/traceevent/event-plugin.c +54 -6

95 lines not on this page · raw message

Signature · 1 line
2.1.4
#16 lore raw

[tip:perf/core] tools lib traceevent: Make plugin options either string or boolean

tools lib traceevent: Make plugin options either string or boolean
Commit-ID:  5dbcfd930e430b33fb4dfded9c1af0cf432772b9
Gitweb:     http://git.kernel.org/tip/5dbcfd930e430b33fb4dfded9c1af0cf432772b9
Author:     Steven Rostedt <[email protected]>
AuthorDate: Tue, 24 Mar 2015 09:57:54 -0400
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 24 Mar 2015 12:20:09 -0300

tools lib traceevent: Make plugin options either string or boolean

When a plugin option is defined, by default it is a boolean (true or false).

If the option is something else, then it needs to set its "value" field to a default string other than NULL (can be just "").

If the value is not set then the option is considered boolean, and the updating of the option value will be handled accordingly.

---
 tools/lib/traceevent/event-parse.h  |  6 +++-
 tools/lib/traceevent/event-plugin.c | 60 +++++++++++++++++++++++++++++++++----
 2 files changed, 59 insertions(+), 7 deletions(-)
tools/lib/traceevent/event-parse.h +5 -1
index 8bd7c6a..8ca1b8e 100644
@@ -116,7 +116,7 @@ struct pevent_plugin_option {
 	char				*name;
 	char				*plugin_alias;
 	char				*description;
-	char				*value;
+	const char			*value;
 	void				*priv;
 	int				set;
 };
@@ -154,6 +154,10 @@ struct pevent_plugin_option {
  *   .plugin_alias is used to give a shorter name to access
  *   the vairable. Useful if a plugin handles more than one event.
  *
+ *   If .value is not set, then it is considered a boolean and only
+ *   .set will be processed. If .value is defined, then it is considered
+ *   a string option and .set will be ignored.
+ *
  * PEVENT_PLUGIN_ALIAS: (optional)
  *   The name to use for finding options (uses filename if not defined)
  */
tools/lib/traceevent/event-plugin.c +54 -6

95 lines not on this page · raw message

#17 lore raw

PATCH 7/9 tools lib traceevent kbuffer: Remove extra update to data pointer in PADDING

In reply to Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

When a event PADDING is hit (a deleted event that is still in the ring buffer), translate_data() sets the length of the padding and also updates the data pointer which is passed back to the caller. This is unneeded because the caller also updates the data pointer with the passed back length. translate_data() should not update the pointer, only set the length.

---
 tools/lib/traceevent/kbuffer-parse.c | 1 -
 1 file changed, 1 deletion(-)
tools/lib/traceevent/kbuffer-parse.c +0 -1
index dcc665228c71..deb3569ab004 100644
@@ -372,7 +372,6 @@ translate_data(struct kbuffer *kbuf, void *data, void **rptr,
 	switch (type_len) {
 	case KBUFFER_TYPE_PADDING:
 		*length = read_4(kbuf, data);
-		data += *length;
 		break;
 
 	case KBUFFER_TYPE_TIME_EXTEND:
Signature · 1 line
2.1.4
#18 lore raw

[tip:perf/core] tools lib traceevent kbuffer: Remove extra update to data pointer in PADDING

tools lib traceevent kbuffer: Remove extra update to data pointer in PADDING
Commit-ID:  c5e691928bf166ac03430e957038b60adba3cf6c
Gitweb:     http://git.kernel.org/tip/c5e691928bf166ac03430e957038b60adba3cf6c
Author:     Steven Rostedt (Red Hat) <[email protected]>
AuthorDate: Tue, 24 Mar 2015 09:57:55 -0400
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 24 Mar 2015 12:22:04 -0300

tools lib traceevent kbuffer: Remove extra update to data pointer in PADDING

When a event PADDING is hit (a deleted event that is still in the ring buffer), translate_data() sets the length of the padding and also updates the data pointer which is passed back to the caller.

This is unneeded because the caller also updates the data pointer with the passed back length. translate_data() should not update the pointer, only set the length.

---
 tools/lib/traceevent/kbuffer-parse.c | 1 -
 1 file changed, 1 deletion(-)
tools/lib/traceevent/kbuffer-parse.c +0 -1
index dcc6652..deb3569 100644
@@ -372,7 +372,6 @@ translate_data(struct kbuffer *kbuf, void *data, void **rptr,
 	switch (type_len) {
 	case KBUFFER_TYPE_PADDING:
 		*length = read_4(kbuf, data);
-		data += *length;
 		break;
 
 	case KBUFFER_TYPE_TIME_EXTEND:
#19 lore raw

PATCH 8/9 tools lib traceevent: Add way to find sub buffer boundary

In reply to Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

For debugging purposes, it may be helpful for the kbuffer library to flag when crossing a sub buffer.

---
 tools/lib/traceevent/kbuffer-parse.c | 11 +++++++++++
 tools/lib/traceevent/kbuffer.h       |  1 +
 2 files changed, 12 insertions(+)
tools/lib/traceevent/kbuffer-parse.c +11 -0
index deb3569ab004..3bcada3ae05a 100644
@@ -729,3 +729,14 @@ void kbuffer_set_old_format(struct kbuffer *kbuf)
 
 	kbuf->next_event = __old_next_event;
 }
+
+/**
+ * kbuffer_start_of_data - return offset of where data starts on subbuffer
+ * @kbuf:	The kbuffer
+ *
+ * Returns the location on the subbuffer where the data starts.
+ */
+int kbuffer_start_of_data(struct kbuffer *kbuf)
+{
+	return kbuf->start;
+}
tools/lib/traceevent/kbuffer.h +1 -0
index c831f64b17a0..03dce757553f 100644
@@ -63,5 +63,6 @@ int kbuffer_missed_events(struct kbuffer *kbuf);
 int kbuffer_subbuffer_size(struct kbuffer *kbuf);
 
 void kbuffer_set_old_format(struct kbuffer *kbuf);
+int kbuffer_start_of_data(struct kbuffer *kbuf);
 
 #endif /* _K_BUFFER_H */
Signature · 1 line
2.1.4
#20 lore raw

[tip:perf/core] tools lib traceevent: Add way to find sub buffer boundary

tools lib traceevent: Add way to find sub buffer boundary
Commit-ID:  82ac952be6348dc072dcfd80a2dcb511d0cd6bea
Gitweb:     http://git.kernel.org/tip/82ac952be6348dc072dcfd80a2dcb511d0cd6bea
Author:     Steven Rostedt (Red Hat) <[email protected]>
AuthorDate: Tue, 24 Mar 2015 09:57:56 -0400
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 24 Mar 2015 12:22:09 -0300

tools lib traceevent: Add way to find sub buffer boundary

For debugging purposes, it may be helpful for the kbuffer library to flag when crossing a sub buffer.

---
 tools/lib/traceevent/kbuffer-parse.c | 11 +++++++++++
 tools/lib/traceevent/kbuffer.h       |  1 +
 2 files changed, 12 insertions(+)
tools/lib/traceevent/kbuffer-parse.c +11 -0
index deb3569..3bcada3 100644
@@ -729,3 +729,14 @@ void kbuffer_set_old_format(struct kbuffer *kbuf)
 
 	kbuf->next_event = __old_next_event;
 }
+
+/**
+ * kbuffer_start_of_data - return offset of where data starts on subbuffer
+ * @kbuf:	The kbuffer
+ *
+ * Returns the location on the subbuffer where the data starts.
+ */
+int kbuffer_start_of_data(struct kbuffer *kbuf)
+{
+	return kbuf->start;
+}
tools/lib/traceevent/kbuffer.h +1 -0
index c831f64..03dce75 100644
@@ -63,5 +63,6 @@ int kbuffer_missed_events(struct kbuffer *kbuf);
 int kbuffer_subbuffer_size(struct kbuffer *kbuf);
 
 void kbuffer_set_old_format(struct kbuffer *kbuf);
+int kbuffer_start_of_data(struct kbuffer *kbuf);
 
 #endif /* _K_BUFFER_H */
#21 lore raw

PATCH 9/9 tools lib traceevent: Free filter tokens in process_filter()

In reply to Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

valgrind showed that the filter token wasn't being freed properly in process_filter().

---
 tools/lib/traceevent/parse-filter.c | 2 ++
 1 file changed, 2 insertions(+)
tools/lib/traceevent/parse-filter.c +2 -0
index b50234402fc2..0144b3d1bb77 100644
@@ -1058,6 +1058,7 @@ process_filter(struct event_format *event, struct filter_arg **parg,
 					*parg = current_op;
 				else
 					*parg = current_exp;
+				free(token);
 				return PEVENT_ERRNO__UNBALANCED_PAREN;
 			}
 			break;
@@ -1168,6 +1169,7 @@ process_filter(struct event_format *event, struct filter_arg **parg,
 
 	*parg = current_op;
 
+	free(token);
 	return 0;
 
  fail_alloc:
Signature · 1 line
2.1.4
#22 lore raw

[tip:perf/core] tools lib traceevent: Free filter tokens in process_filter()

tools lib traceevent: Free filter tokens in process_filter()
Commit-ID:  e1644aae4589274223c1ab9072ddbda98dd97f6a
Gitweb:     http://git.kernel.org/tip/e1644aae4589274223c1ab9072ddbda98dd97f6a
Author:     Steven Rostedt (Red Hat) <[email protected]>
AuthorDate: Tue, 24 Mar 2015 09:57:57 -0400
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 24 Mar 2015 12:23:03 -0300

tools lib traceevent: Free filter tokens in process_filter()

valgrind showed that the filter token wasn't being freed properly in process_filter().

---
 tools/lib/traceevent/parse-filter.c | 2 ++
 1 file changed, 2 insertions(+)
tools/lib/traceevent/parse-filter.c +2 -0
index b502344..0144b3d 100644
@@ -1058,6 +1058,7 @@ process_filter(struct event_format *event, struct filter_arg **parg,
 					*parg = current_op;
 				else
 					*parg = current_exp;
+				free(token);
 				return PEVENT_ERRNO__UNBALANCED_PAREN;
 			}
 			break;
@@ -1168,6 +1169,7 @@ process_filter(struct event_format *event, struct filter_arg **parg,
 
 	*parg = current_op;
 
+	free(token);
 	return 0;
 
  fail_alloc:
#23 lore raw
In reply to Steven Rostedt

Em Tue, Mar 24, 2015 at 09:57:48AM -0400, Steven Rostedt escreveu:

Quoted 5 lines

Arnaldo,

I took a look at the differences between what I have in trace-cmd and what is sitting in tools/lib/traceevent, and I put together a patch set that brings in fixes and updates to libtraceevent.

Thanks, will process now.

Quoted 28 lines

-- Steve

Local SHA1: bd9d9602719a24d0843bb04e52b6bf98d9e90881

Josef Bacik (1):
tools lib traceevent: Handle NULL comm name

Steven Rostedt (1):
tools lib traceevent: Make plugin options either string or boolean

Steven Rostedt (Red Hat) (7):
tools lib traceevent: Copy trace_clock and free it
tools lib traceevent: Handle '%z' in bprint format
tools lib traceevent: Add pevent_data_pid_from_comm()
tools lib traceevent: Fix whitespace error
tools lib traceevent kbuffer: Remove extra update to data pointer in PADDING tools lib traceevent: Add way to find sub buffer boundary
tools lib traceevent: Free filter tokens in process_filter()

----

 tools/lib/traceevent/event-parse.c   | 109 ++++++++++++++++++++++++++++++++++-
 tools/lib/traceevent/event-parse.h   |  13 ++++-
 tools/lib/traceevent/event-plugin.c  |  60 +++++++++++++++++--
 tools/lib/traceevent/kbuffer-parse.c |  12 +++-
 tools/lib/traceevent/kbuffer.h       |   1 +
 tools/lib/traceevent/parse-filter.c  |   2 +
 6 files changed, 185 insertions(+), 12 deletions(-)
#24 lore raw
Quoted 8 lines · On Tue, 24 Mar 2015 11:57:27 -0300 Arnaldo Carvalho de Melo <[email protected]>…

Em Tue, Mar 24, 2015 at 09:57:48AM -0400, Steven Rostedt escreveu:

Quoted 5 lines

Arnaldo,

I took a look at the differences between what I have in trace-cmd and what is sitting in tools/lib/traceevent, and I put together a patch set that brings in fixes and updates to libtraceevent.

Thanks, will process now.

I just added this one too...

-- Steve

Quoted 1 line

From fe80b2e9396fa6b8dc66a6dfd4c7a426e1685424 Mon Sep 17 00:00:00 2001

From: Guilherme Cox <[email protected]>
Date: Fri, 20 Feb 2015 19:10:34 -0500
Subject: [PATCH] tools lib traceevent: Zero should not be considered "not
found" in eval_flag()

Guilherme Cox found that:
There is, however, a potential bug if there is an item with code zero that is not the first one in the symbol list, since eval_flag(..) returns 0 when it doesn't find anything.

---
 tools/lib/traceevent/event-parse.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
tools/lib/traceevent/event-parse.c +5 -5
index aff743710001..f2fb50141703 100644
@@ -3576,7 +3576,7 @@ static const struct flag flags[] = {
 	{ "HRTIMER_RESTART", 1 },
 };
 
-static unsigned long long eval_flag(const char *flag)
+static long long eval_flag(const char *flag)
 {
 	int i;
 
@@ -3592,7 +3592,7 @@ static unsigned long long eval_flag(const char *flag)
 		if (strcmp(flags[i].name, flag) == 0)
 			return flags[i].value;
 
-	return 0;
+	return -1LL;
 }
 
 static void print_str_to_seq(struct trace_seq *s, const char *format,
@@ -3666,7 +3666,7 @@ static void print_str_arg(struct trace_seq *s, void *data, int size,
 	struct print_flag_sym *flag;
 	struct format_field *field;
 	struct printk_map *printk;
-	unsigned long long val, fval;
+	long long val, fval;
 	unsigned long addr;
 	char *str;
 	unsigned char *hex;
@@ -3725,11 +3725,11 @@ static void print_str_arg(struct trace_seq *s, void *data, int size,
 		print = 0;
 		for (flag = arg->flags.flags; flag; flag = flag->next) {
 			fval = eval_flag(flag->value);
-			if (!val && !fval) {
+			if (!val && fval < 0) {
 				print_str_to_seq(s, format, len_arg, flag->str);
 				break;
 			}
-			if (fval && (val & fval) == fval) {
+			if (fval > 0 && (val & fval) == fval) {
 				if (print && arg->flags.delim)
 					trace_seq_puts(s, arg->flags.delim);
 				print_str_to_seq(s, format, len_arg, flag->str);
Signature · 1 line
1.8.3.1
#25 lore raw

Em Tue, Mar 24, 2015 at 11:04:28AM -0400, Steven Rostedt escreveu:

Quoted 6 lines
Quoted 3 lines · On Tue, 24 Mar 2015 11:57:27 -0300 Arnaldo Carvalho de Melo <[email protected]>…

Em Tue, Mar 24, 2015 at 09:57:48AM -0400, Steven Rostedt escreveu:

Quoted 2 lines

Arnaldo,

Quoted 7 lines
Quoted 5 lines
Quoted 3 lines

I took a look at the differences between what I have in trace-cmd and what is sitting in tools/lib/traceevent, and I put together a patch set that brings in fixes and updates to libtraceevent.

Thanks, will process now.

I just added this one too...

ok, adding this one as well.

Quoted 66 lines

-- Steve

From fe80b2e9396fa6b8dc66a6dfd4c7a426e1685424 Mon Sep 17 00:00:00 2001
From: Guilherme Cox <[email protected]>
Date: Fri, 20 Feb 2015 19:10:34 -0500
Subject: [PATCH] tools lib traceevent: Zero should not be considered "not
found" in eval_flag()

Guilherme Cox found that:
There is, however, a potential bug if there is an item with code zero that is not the first one in the symbol list, since eval_flag(..) returns 0 when it doesn't find anything.

---
 tools/lib/traceevent/event-parse.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
tools/lib/traceevent/event-parse.c +5 -5
index aff743710001..f2fb50141703 100644
@@ -3576,7 +3576,7 @@ static const struct flag flags[] = {
 	{ "HRTIMER_RESTART", 1 },
 };
 
-static unsigned long long eval_flag(const char *flag)
+static long long eval_flag(const char *flag)
 {
 	int i;
 
@@ -3592,7 +3592,7 @@ static unsigned long long eval_flag(const char *flag)
 		if (strcmp(flags[i].name, flag) == 0)
 			return flags[i].value;
 
-	return 0;
+	return -1LL;
 }
 
 static void print_str_to_seq(struct trace_seq *s, const char *format,
@@ -3666,7 +3666,7 @@ static void print_str_arg(struct trace_seq *s, void *data, int size,
 	struct print_flag_sym *flag;
 	struct format_field *field;
 	struct printk_map *printk;
-	unsigned long long val, fval;
+	long long val, fval;
 	unsigned long addr;
 	char *str;
 	unsigned char *hex;
@@ -3725,11 +3725,11 @@ static void print_str_arg(struct trace_seq *s, void *data, int size,
 		print = 0;
 		for (flag = arg->flags.flags; flag; flag = flag->next) {
 			fval = eval_flag(flag->value);
-			if (!val && !fval) {
+			if (!val && fval < 0) {
 				print_str_to_seq(s, format, len_arg, flag->str);
 				break;
 			}
-			if (fval && (val & fval) == fval) {
+			if (fval > 0 && (val & fval) == fval) {
 				if (print && arg->flags.delim)
 					trace_seq_puts(s, arg->flags.delim);
 				print_str_to_seq(s, format, len_arg, flag->str);
Signature · 1 line
1.8.3.1
#26 lore raw
In reply to Steven Rostedt

Em Tue, Mar 24, 2015 at 11:04:28AM -0400, Steven Rostedt escreveu:

Quoted 6 lines
Quoted 3 lines · On Tue, 24 Mar 2015 11:57:27 -0300 Arnaldo Carvalho de Melo <[email protected]>…

Em Tue, Mar 24, 2015 at 09:57:48AM -0400, Steven Rostedt escreveu:

Quoted 2 lines

Arnaldo,

Quoted 7 lines
Quoted 5 lines
Quoted 3 lines

I took a look at the differences between what I have in trace-cmd and what is sitting in tools/lib/traceevent, and I put together a patch set that brings in fixes and updates to libtraceevent.

Thanks, will process now.

I just added this one too...

ok, but who is the author? Is Guilherme just the reporter or was it him that sent the patch but hasn't provided a S-o-B?

- Arnaldo

Quoted 66 lines

-- Steve

From fe80b2e9396fa6b8dc66a6dfd4c7a426e1685424 Mon Sep 17 00:00:00 2001
From: Guilherme Cox <[email protected]>
Date: Fri, 20 Feb 2015 19:10:34 -0500
Subject: [PATCH] tools lib traceevent: Zero should not be considered "not
found" in eval_flag()

Guilherme Cox found that:
There is, however, a potential bug if there is an item with code zero that is not the first one in the symbol list, since eval_flag(..) returns 0 when it doesn't find anything.

---
 tools/lib/traceevent/event-parse.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
tools/lib/traceevent/event-parse.c +5 -5
index aff743710001..f2fb50141703 100644
@@ -3576,7 +3576,7 @@ static const struct flag flags[] = {
 	{ "HRTIMER_RESTART", 1 },
 };
 
-static unsigned long long eval_flag(const char *flag)
+static long long eval_flag(const char *flag)
 {
 	int i;
 
@@ -3592,7 +3592,7 @@ static unsigned long long eval_flag(const char *flag)
 		if (strcmp(flags[i].name, flag) == 0)
 			return flags[i].value;
 
-	return 0;
+	return -1LL;
 }
 
 static void print_str_to_seq(struct trace_seq *s, const char *format,
@@ -3666,7 +3666,7 @@ static void print_str_arg(struct trace_seq *s, void *data, int size,
 	struct print_flag_sym *flag;
 	struct format_field *field;
 	struct printk_map *printk;
-	unsigned long long val, fval;
+	long long val, fval;
 	unsigned long addr;
 	char *str;
 	unsigned char *hex;
@@ -3725,11 +3725,11 @@ static void print_str_arg(struct trace_seq *s, void *data, int size,
 		print = 0;
 		for (flag = arg->flags.flags; flag; flag = flag->next) {
 			fval = eval_flag(flag->value);
-			if (!val && !fval) {
+			if (!val && fval < 0) {
 				print_str_to_seq(s, format, len_arg, flag->str);
 				break;
 			}
-			if (fval && (val & fval) == fval) {
+			if (fval > 0 && (val & fval) == fval) {
 				if (print && arg->flags.delim)
 					trace_seq_puts(s, arg->flags.delim);
 				print_str_to_seq(s, format, len_arg, flag->str);
Signature · 1 line
1.8.3.1
#27 lore raw
Quoted 11 lines · On Tue, 24 Mar 2015 12:26:37 -0300 Arnaldo Carvalho de Melo <[email protected]>…

Em Tue, Mar 24, 2015 at 11:04:28AM -0400, Steven Rostedt escreveu:

Quoted 10 lines
Quoted 7 lines · On Tue, 24 Mar 2015 11:57:27 -0300 Arnaldo Carvalho de Melo <[email protected]>…

Em Tue, Mar 24, 2015 at 09:57:48AM -0400, Steven Rostedt escreveu:

Quoted 5 lines

Arnaldo,

I took a look at the differences between what I have in trace-cmd and what is sitting in tools/lib/traceevent, and I put together a patch set that brings in fixes and updates to libtraceevent.

Quoted 7 lines
Quoted 3 lines
Quoted 1 line

Thanks, will process now.

I just added this one too...

ok, but who is the author? Is Guilherme just the reporter or was it him that sent the patch but hasn't provided a S-o-B?

He just reported it.

-- Steve

#28 lore raw
Quoted 16 lines · On Tue, 24 Mar 2015 11:44:06 -0400 Steven Rostedt <[email protected]> wrote:
Quoted 13 lines · On Tue, 24 Mar 2015 12:26:37 -0300 Arnaldo Carvalho de Melo <[email protected]>…

Em Tue, Mar 24, 2015 at 11:04:28AM -0400, Steven Rostedt escreveu:

Quoted 12 lines
Quoted 8 lines · On Tue, 24 Mar 2015 11:57:27 -0300 Arnaldo Carvalho de Melo <[email protected]>…

Em Tue, Mar 24, 2015 at 09:57:48AM -0400, Steven Rostedt escreveu:

Quoted 5 lines

Arnaldo,

I took a look at the differences between what I have in trace-cmd and what is sitting in tools/lib/traceevent, and I put together a patch set that brings in fixes and updates to libtraceevent.

Thanks, will process now.

Quoted 7 lines
Quoted 5 lines
Quoted 1 line

I just added this one too...

ok, but who is the author? Is Guilherme just the reporter or was it him that sent the patch but hasn't provided a S-o-B?

He just reported it.

Actually, hold off on applying it. I want to test it a bit more. I really just applied it today :-) and sent it to you. It was only in a quilt queue before that. Not my git repo.

-- Steve

#29 lore raw

Em Tue, Mar 24, 2015 at 11:45:26AM -0400, Steven Rostedt escreveu:

Quoted 15 lines
Quoted 12 lines · On Tue, 24 Mar 2015 11:44:06 -0400 Steven Rostedt <[email protected]> wrote:
Quoted 11 lines · On Tue, 24 Mar 2015 12:26:37 -0300 Arnaldo Carvalho de Melo <[email protected]>…

Em Tue, Mar 24, 2015 at 11:04:28AM -0400, Steven Rostedt escreveu:

Quoted 10 lines
Quoted 6 lines · On Tue, 24 Mar 2015 11:57:27 -0300 Arnaldo Carvalho de Melo <[email protected]>…

Em Tue, Mar 24, 2015 at 09:57:48AM -0400, Steven Rostedt escreveu:

Quoted 3 lines

I took a look at the differences between what I have in trace-cmd and what is sitting in tools/lib/traceevent, and I put together a patch set that brings in fixes and updates to libtraceevent.

Thanks, will process now.

I just added this one too...

Quoted 8 lines
Quoted 4 lines
Quoted 2 lines

ok, but who is the author? Is Guilherme just the reporter or was it him that sent the patch but hasn't provided a S-o-B?

He just reported it.

Actually, hold off on applying it. I want to test it a bit more. I really just applied it today :-) and sent it to you. It was only in a quilt queue before that. Not my git repo.

Ok, its just that if it comes like that it will appear as being authored by him, no?

- Arnaldo

#30 lore raw
Quoted 6 lines · On Tue, 24 Mar 2015 12:51:41 -0300 Arnaldo Carvalho de Melo <[email protected]>…
Quoted 3 lines

Actually, hold off on applying it. I want to test it a bit more. I really just applied it today :-) and sent it to you. It was only in a quilt queue before that. Not my git repo.

Ok, its just that if it comes like that it will appear as being authored by him, no?

Crap, you're right. Thanks for pointing that out to me. Instead of cut and pasting his text and his email address, and creating a change log from scratch, I saved the email and appending my patch to it, then modified the rest. But I never took out the "From:".

OK, I'll fix that too. Thanks for mentioning it.

-- Steve