[PATCH v2 0/4] trace-cmd: Fixes for four minor bugs

[PATCH v2 0/4] trace-cmd: Fixes for four minor bugs

[Michael Sartain]

Thanks Steve.
 -Mike

v2: check malloc size, use UL suffix, and unsigned parameter

Michael Sartain (4):
  trace-cmd: Fix incorrect malloc size arg: *item instead of item
  trace-cmd: Fix NULL pointer being passed to memcpy
  trace-cmd: Add UL suffix to MISSING_EVENTS since ints shouldn't be
    left shifted by 31
  trace-cmd: Use unsigned values in Hsieh's trace_hash fast hash
    function

 kbuffer-parse.c    | 4 ++--
 trace-dialog.c     | 2 +-
 trace-hash-local.h | 4 ++--
 trace-output.c     | 6 +++++-
 4 files changed, 10 insertions(+), 6 deletions(-)

-- 
2.14.2

[PATCH v2 1/4] trace-cmd: Fix incorrect malloc size arg: *item instead of item

[Michael Sartain]

Signed-off-by: Michael Sartain <mikesart@fastmail.com>
---
 trace-dialog.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/trace-dialog.c b/trace-dialog.c
index b5776cc..87e597a 100644
--- a/trace-dialog.c
+++ b/trace-dialog.c
@@ -97,7 +97,7 @@ static void push_cursor(GdkCursor *cursor)
 {
 	struct cursor_stack *item;
 
-	item = malloc_or_die(sizeof(item));
+	item = malloc_or_die(sizeof(*item));
 	item->next = cursor_stack;
 	cursor_stack = item;
 	item->cursor = cursor;
-- 
2.14.2

[PATCH v2 2/4] trace-cmd: Fix NULL pointer being passed to memcpy

[Michael Sartain]

Signed-off-by: Michael Sartain <mikesart@fastmail.com>
---
 trace-output.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/trace-output.c b/trace-output.c
index bfe6331..bbb1637 100644
--- a/trace-output.c
+++ b/trace-output.c
@@ -929,7 +929,11 @@ tracecmd_add_option(struct tracecmd_output *handle,
 		free(option);
 		return NULL;
 	}
-	memcpy(option->data, data, size);
+
+	/* Some IDs (like TRACECMD_OPTION_TRACECLOCK) pass 0 / NULL data */
+	if (size)
+		memcpy(option->data, data, size);
+
 	list_add_tail(&option->list, &handle->options);
 
 	return option;
-- 
2.14.2

[PATCH v2 3/4] trace-cmd: Add UL suffix to MISSING_EVENTS since ints shouldn't be left shifted by 31

[Michael Sartain]

Signed-off-by: Michael Sartain <mikesart@fastmail.com>
---
 kbuffer-parse.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kbuffer-parse.c b/kbuffer-parse.c
index 4e6e95e..593d0ab 100644
--- a/kbuffer-parse.c
+++ b/kbuffer-parse.c
@@ -24,8 +24,8 @@
 
 #include "kbuffer.h"
 
-#define MISSING_EVENTS (1 << 31)
-#define MISSING_STORED (1 << 30)
+#define MISSING_EVENTS (1UL << 31)
+#define MISSING_STORED (1UL << 30)
 
 #define COMMIT_MASK ((1 << 27) - 1)
 
-- 
2.14.2

[PATCH v2 4/4] trace-cmd: Use unsigned values in Hsieh's trace_hash fast hash function

[Michael Sartain]

Signed-off-by: Michael Sartain <mikesart@fastmail.com>
---
 trace-hash-local.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/trace-hash-local.h b/trace-hash-local.h
index b2a1002..7c822a8 100644
--- a/trace-hash-local.h
+++ b/trace-hash-local.h
@@ -20,9 +20,9 @@
 #ifndef _TRACE_HASH_LOCAL_H
 #define _TRACE_HASH_LOCAL_H
 
-static inline unsigned int trace_hash(int val)
+static inline unsigned int trace_hash(unsigned int val)
 {
-	int hash, tmp;
+	unsigned int hash, tmp;
 
 	hash = 12546869;	/* random prime */
 
-- 
2.14.2

[PATCH 00/10] tools lib traceveent: Pull updates from trace-cmd library

[Steven Rostedt]

I went through the commits that were added to event-parse in trace-cmd
and synced them up for what is in tools/lib/traceevent. These are
patches that are missing from the kernel tree.

Federico Vaga (1):
      lib traceevent: Use asprintf when possible

Jan Kiszka (1):
      lib traceevent: Print value of unknown symbolic fields

Michael Sartain (2):
      lib, traceevent: Fix bad force_token escape sequence
      lib traceevent: Add UL suffix to MISSING_EVENTS

Steven Rostedt (VMware) (5):
      lib traceevent: Show value of flags that have not been parsed
      lib traceevent: Simplify pointer print logic and fix %pF
      lib traceevent: Handle new pointer processing of bprint strings
      lib traceevent: Show contents (in hex) of data of unrecognized type records
      lib traceevent: Fix get_field_str() for dynamic strings

Taeung Song (1):
      lib traceeevent: Fix missing break in FALSE case of pevent_filter_clear_trivial()

----
 tools/lib/traceevent/event-parse.c   | 62 +++++++++++++++++++++++++++++-------
 tools/lib/traceevent/event-plugin.c  | 24 ++++++--------
 tools/lib/traceevent/kbuffer-parse.c |  4 +--
 tools/lib/traceevent/parse-filter.c  | 22 ++++++++-----
 4 files changed, 76 insertions(+), 36 deletions(-)

[PATCH 01/10] lib, traceevent: Fix bad force_token escape sequence

[Steven Rostedt]

[-- Attachment #1: 0001-lib-traceevent-Fix-bad-force_token-escape-sequence.patch --]
[-- Type: text/plain, Size: 1090 bytes --]

From: Michael Sartain <mikesart@fastmail.com>

Older kernels have a bug that creates invalid symbols. event-parse.c
handles them by replacing them with a "%s" token. But the fix included
an extra backslash, and "\%s" was added incorrectly.

Link: http://lkml.kernel.org/r/d320000d37c10ce0912851e1fb78d1e0c946bcd9.1497486273.git.mikesart@fastmail.com

Signed-off-by: Michael Sartain <mikesart@fastmail.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 tools/lib/traceevent/event-parse.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 7ce724fc0544..0bc1a6df8a27 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -1094,7 +1094,7 @@ static enum event_type __read_token(char **tok)
 		if (strcmp(*tok, "LOCAL_PR_FMT") == 0) {
 			free(*tok);
 			*tok = NULL;
-			return force_token("\"\%s\" ", tok);
+			return force_token("\"%s\" ", tok);
 		} else if (strcmp(*tok, "STA_PR_FMT") == 0) {
 			free(*tok);
 			*tok = NULL;
-- 
2.13.2

[PATCH 02/10] lib traceevent: Show value of flags that have not been parsed

[Steven Rostedt]

[-- Attachment #1: 0002-lib-traceevent-Show-value-of-flags-that-have-not-bee.patch --]
[-- Type: text/plain, Size: 1003 bytes --]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

If the value contains bits that are not defined by print_flags() helper,
then show the remaining bits. This aligns with the functionality of the
kernel.

Link: http://lkml.kernel.org/r/e60c889f-55e7-4ee8-0e50-151e435ffd8c@siemens.com

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 tools/lib/traceevent/event-parse.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 0bc1a6df8a27..96c9c0b33423 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -3970,6 +3970,11 @@ static void print_str_arg(struct trace_seq *s, void *data, int size,
 				val &= ~fval;
 			}
 		}
+		if (val) {
+			if (print && arg->flags.delim)
+				trace_seq_puts(s, arg->flags.delim);
+			trace_seq_printf(s, "0x%llx", val);
+		}
 		break;
 	case PRINT_SYMBOL:
 		val = eval_num_arg(data, size, event, arg->symbol.field);
-- 
2.13.2

[tip:perf/core] tools lib traceevent: Show value of flags that have not been parsed

[tip-bot for Steven Rostedt (VMware)]

Commit-ID:  3df76c9a8167ffff1588516fc74b980cde664efe
Gitweb:     https://git.kernel.org/tip/3df76c9a8167ffff1588516fc74b980cde664efe
Author:     Steven Rostedt (VMware) <rostedt@goodmis.org>
AuthorDate: Thu, 11 Jan 2018 19:47:43 -0500
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 17 Jan 2018 10:21:45 -0300

tools lib traceevent: Show value of flags that have not been parsed

If the value contains bits that are not defined by print_flags() helper,
then show the remaining bits. This aligns with the functionality of the
kernel.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Link: http://lkml.kernel.org/r/e60c889f-55e7-4ee8-0e50-151e435ffd8c@siemens.com
Link: http://lkml.kernel.org/r/20180112004821.976225232@goodmis.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/traceevent/event-parse.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 0bc1a6d..96c9c0b 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -3970,6 +3970,11 @@ static void print_str_arg(struct trace_seq *s, void *data, int size,
 				val &= ~fval;
 			}
 		}
+		if (val) {
+			if (print && arg->flags.delim)
+				trace_seq_puts(s, arg->flags.delim);
+			trace_seq_printf(s, "0x%llx", val);
+		}
 		break;
 	case PRINT_SYMBOL:
 		val = eval_num_arg(data, size, event, arg->symbol.field);

[PATCH 03/10] lib traceevent: Print value of unknown symbolic fields

[Steven Rostedt]

[-- Attachment #1: 0003-lib-traceevent-Print-value-of-unknown-symbolic-field.patch --]
[-- Type: text/plain, Size: 804 bytes --]

From: Jan Kiszka <jan.kiszka@siemens.com>

Aligns trace-cmd with the behavior of the kernel.

Link: http://lkml.kernel.org/r/e60c889f-55e7-4ee8-0e50-151e435ffd8c@siemens.com

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 tools/lib/traceevent/event-parse.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 96c9c0b33423..87757eabbb08 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -3985,6 +3985,8 @@ static void print_str_arg(struct trace_seq *s, void *data, int size,
 				break;
 			}
 		}
+		if (!flag)
+			trace_seq_printf(s, "0x%llx", val);
 		break;
 	case PRINT_HEX:
 	case PRINT_HEX_STR:
-- 
2.13.2

[tip:perf/core] tools lib traceevent: Print value of unknown symbolic fields

[tip-bot for Jan Kiszka]

Commit-ID:  d63444739bee6acfa9a834515da17f9cec544505
Gitweb:     https://git.kernel.org/tip/d63444739bee6acfa9a834515da17f9cec544505
Author:     Jan Kiszka <jan.kiszka@siemens.com>
AuthorDate: Thu, 11 Jan 2018 19:47:44 -0500
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 17 Jan 2018 10:21:57 -0300

tools lib traceevent: Print value of unknown symbolic fields

Aligns trace-cmd with the behavior of the kernel.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Link: http://lkml.kernel.org/r/e60c889f-55e7-4ee8-0e50-151e435ffd8c@siemens.com
Link: http://lkml.kernel.org/r/20180112004822.118332436@goodmis.org
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/traceevent/event-parse.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 96c9c0b..87757ea 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -3985,6 +3985,8 @@ static void print_str_arg(struct trace_seq *s, void *data, int size,
 				break;
 			}
 		}
+		if (!flag)
+			trace_seq_printf(s, "0x%llx", val);
 		break;
 	case PRINT_HEX:
 	case PRINT_HEX_STR:

[PATCH 04/10] lib traceevent: Simplify pointer print logic and fix %pF

[Steven Rostedt]

[-- Attachment #1: 0004-lib-traceevent-Simplify-pointer-print-logic-and-fix-.patch --]
[-- Type: text/plain, Size: 1700 bytes --]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

When processing %pX in pretty_print(), simplify the logic slightly by
incrementing the ptr to the format string if isalnum(ptr[1]) is true. This
follows the logic a bit more closely to what is in the kernel.

Also, this fixes a small bug where %pF was not giving the offset of the
function.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 tools/lib/traceevent/event-parse.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 87757eabbb08..8757dd64e42c 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -4956,21 +4956,22 @@ static void pretty_print(struct trace_seq *s, void *data, int size, struct event
 				else
 					ls = 2;
 
-				if (*(ptr+1) == 'F' || *(ptr+1) == 'f' ||
-				    *(ptr+1) == 'S' || *(ptr+1) == 's') {
+				if (isalnum(ptr[1]))
 					ptr++;
+
+				if (*ptr == 'F' || *ptr == 'f' ||
+				    *ptr == 'S' || *ptr == 's') {
 					show_func = *ptr;
-				} else if (*(ptr+1) == 'M' || *(ptr+1) == 'm') {
-					print_mac_arg(s, *(ptr+1), data, size, event, arg);
-					ptr++;
+				} else if (*ptr == 'M' || *ptr == 'm') {
+					print_mac_arg(s, *ptr, data, size, event, arg);
 					arg = arg->next;
 					break;
-				} else if (*(ptr+1) == 'I' || *(ptr+1) == 'i') {
+				} else if (*ptr == 'I' || *ptr == 'i') {
 					int n;
 
-					n = print_ip_arg(s, ptr+1, data, size, event, arg);
+					n = print_ip_arg(s, ptr, data, size, event, arg);
 					if (n > 0) {
-						ptr += n;
+						ptr += n - 1;
 						arg = arg->next;
 						break;
 					}
-- 
2.13.2

[tip:perf/core] tools lib traceevent: Simplify pointer print logic and fix %pF

[tip-bot for Steven Rostedt (VMware)]

Commit-ID:  38d70b7ca1769f26c0b79f3c08ff2cc949712b59
Gitweb:     https://git.kernel.org/tip/38d70b7ca1769f26c0b79f3c08ff2cc949712b59
Author:     Steven Rostedt (VMware) <rostedt@goodmis.org>
AuthorDate: Thu, 11 Jan 2018 19:47:45 -0500
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 17 Jan 2018 10:22:03 -0300

tools lib traceevent: Simplify pointer print logic and fix %pF

When processing %pX in pretty_print(), simplify the logic slightly by
incrementing the ptr to the format string if isalnum(ptr[1]) is true.
This follows the logic a bit more closely to what is in the kernel.

Also, this fixes a small bug where %pF was not giving the offset of the
function.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Link: http://lkml.kernel.org/r/20180112004822.260262257@goodmis.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/traceevent/event-parse.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 87757ea..8757dd6 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -4956,21 +4956,22 @@ static void pretty_print(struct trace_seq *s, void *data, int size, struct event
 				else
 					ls = 2;
 
-				if (*(ptr+1) == 'F' || *(ptr+1) == 'f' ||
-				    *(ptr+1) == 'S' || *(ptr+1) == 's') {
+				if (isalnum(ptr[1]))
 					ptr++;
+
+				if (*ptr == 'F' || *ptr == 'f' ||
+				    *ptr == 'S' || *ptr == 's') {
 					show_func = *ptr;
-				} else if (*(ptr+1) == 'M' || *(ptr+1) == 'm') {
-					print_mac_arg(s, *(ptr+1), data, size, event, arg);
-					ptr++;
+				} else if (*ptr == 'M' || *ptr == 'm') {
+					print_mac_arg(s, *ptr, data, size, event, arg);
 					arg = arg->next;
 					break;
-				} else if (*(ptr+1) == 'I' || *(ptr+1) == 'i') {
+				} else if (*ptr == 'I' || *ptr == 'i') {
 					int n;
 
-					n = print_ip_arg(s, ptr+1, data, size, event, arg);
+					n = print_ip_arg(s, ptr, data, size, event, arg);
 					if (n > 0) {
-						ptr += n;
+						ptr += n - 1;
 						arg = arg->next;
 						break;
 					}

[PATCH 05/10] lib traceevent: Handle new pointer processing of bprint strings

[Steven Rostedt]

[-- Attachment #1: 0005-lib-traceevent-Handle-new-pointer-processing-of-bpri.patch --]
[-- Type: text/plain, Size: 2323 bytes --]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

The Linux kernel printf() has some extended use cases that dereference the
pointer. This is dangerouse for tracing because the pointer that is
dereferenced can change or even be unmapped. It also causes issues when the
trace data is extracted, because user space does not have access to the
contents of the pointer even if it still exists.

To handle this, the kernel was updated to process these dereferenced
pointers at the time they are recorded, and not post processed. Now they
exist in the tracing buffer, and no dereference is needed at the time of
reading the trace.

The event parsing library needs to handle this new case.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 tools/lib/traceevent/event-parse.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 8757dd64e42c..344a034a8fbc 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -4300,6 +4300,26 @@ static struct print_arg *make_bprint_args(char *fmt, void *data, int size, struc
 				goto process_again;
 			case 'p':
 				ls = 1;
+				if (isalnum(ptr[1])) {
+					ptr++;
+					/* Check for special pointers */
+					switch (*ptr) {
+					case 's':
+					case 'S':
+					case 'f':
+					case 'F':
+						break;
+					default:
+						/*
+						 * Older kernels do not process
+						 * dereferenced pointers.
+						 * Only process if the pointer
+						 * value is a printable.
+						 */
+						if (isprint(*(char *)bptr))
+							goto process_string;
+					}
+				}
 				/* fall through */
 			case 'd':
 			case 'u':
@@ -4352,6 +4372,7 @@ static struct print_arg *make_bprint_args(char *fmt, void *data, int size, struc
 
 				break;
 			case 's':
+ process_string:
 				arg = alloc_arg();
 				if (!arg) {
 					do_warning_event(event, "%s(%d): not enough memory!",
@@ -4959,6 +4980,11 @@ static void pretty_print(struct trace_seq *s, void *data, int size, struct event
 				if (isalnum(ptr[1]))
 					ptr++;
 
+				if (arg->type == PRINT_BSTRING) {
+					trace_seq_puts(s, arg->string.string);
+					break;
+				}
+
 				if (*ptr == 'F' || *ptr == 'f' ||
 				    *ptr == 'S' || *ptr == 's') {
 					show_func = *ptr;
-- 
2.13.2

[tip:perf/core] tools lib traceevent: Handle new pointer processing of bprint strings

[tip-bot for Steven Rostedt (VMware)]

Commit-ID:  37db96bb49629681cb839d7304a70524fe10f969
Gitweb:     https://git.kernel.org/tip/37db96bb49629681cb839d7304a70524fe10f969
Author:     Steven Rostedt (VMware) <rostedt@goodmis.org>
AuthorDate: Thu, 11 Jan 2018 19:47:46 -0500
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 17 Jan 2018 10:22:08 -0300

tools lib traceevent: Handle new pointer processing of bprint strings

The Linux kernel printf() has some extended use cases that dereference
the pointer. This is dangerouse for tracing because the pointer that is
dereferenced can change or even be unmapped. It also causes issues when
the trace data is extracted, because user space does not have access to
the contents of the pointer even if it still exists.

To handle this, the kernel was updated to process these dereferenced
pointers at the time they are recorded, and not post processed. Now they
exist in the tracing buffer, and no dereference is needed at the time of
reading the trace.

The event parsing library needs to handle this new case.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Link: http://lkml.kernel.org/r/20180112004822.403349289@goodmis.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/traceevent/event-parse.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 8757dd6..344a034 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -4300,6 +4300,26 @@ static struct print_arg *make_bprint_args(char *fmt, void *data, int size, struc
 				goto process_again;
 			case 'p':
 				ls = 1;
+				if (isalnum(ptr[1])) {
+					ptr++;
+					/* Check for special pointers */
+					switch (*ptr) {
+					case 's':
+					case 'S':
+					case 'f':
+					case 'F':
+						break;
+					default:
+						/*
+						 * Older kernels do not process
+						 * dereferenced pointers.
+						 * Only process if the pointer
+						 * value is a printable.
+						 */
+						if (isprint(*(char *)bptr))
+							goto process_string;
+					}
+				}
 				/* fall through */
 			case 'd':
 			case 'u':
@@ -4352,6 +4372,7 @@ static struct print_arg *make_bprint_args(char *fmt, void *data, int size, struc
 
 				break;
 			case 's':
+ process_string:
 				arg = alloc_arg();
 				if (!arg) {
 					do_warning_event(event, "%s(%d): not enough memory!",
@@ -4959,6 +4980,11 @@ static void pretty_print(struct trace_seq *s, void *data, int size, struct event
 				if (isalnum(ptr[1]))
 					ptr++;
 
+				if (arg->type == PRINT_BSTRING) {
+					trace_seq_puts(s, arg->string.string);
+					break;
+				}
+
 				if (*ptr == 'F' || *ptr == 'f' ||
 				    *ptr == 'S' || *ptr == 's') {
 					show_func = *ptr;

[PATCH 06/10] lib traceevent: Show contents (in hex) of data of unrecognized type records

[Steven Rostedt]

[-- Attachment #1: 0006-lib-traceevent-Show-contents-in-hex-of-data-of-unrec.patch --]
[-- Type: text/plain, Size: 1225 bytes --]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

When a record has an unrecognized type, an error message is reported, but it
would also be helpful to see the contents of that record. At least show what
it is in hex, instead of just showing a blank line.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 tools/lib/traceevent/event-parse.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 344a034a8fbc..e5f2acbb70cc 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -5566,8 +5566,14 @@ void pevent_print_event(struct pevent *pevent, struct trace_seq *s,
 
 	event = pevent_find_event_by_record(pevent, record);
 	if (!event) {
-		do_warning("ug! no event found for type %d",
-			   trace_parse_common_type(pevent, record->data));
+		int i;
+		int type = trace_parse_common_type(pevent, record->data);
+
+		do_warning("ug! no event found for type %d", type);
+		trace_seq_printf(s, "[UNKNOWN TYPE %d]", type);
+		for (i = 0; i < record->size; i++)
+			trace_seq_printf(s, " %02x",
+					 ((unsigned char *)record->data)[i]);
 		return;
 	}
 
-- 
2.13.2

[tip:perf/core] tools lib traceevent: Show contents (in hex) of data of unrecognized type records

[tip-bot for Steven Rostedt (VMware)]

Commit-ID:  e877372880f72399323e433187cce2bfbea40263
Gitweb:     https://git.kernel.org/tip/e877372880f72399323e433187cce2bfbea40263
Author:     Steven Rostedt (VMware) <rostedt@goodmis.org>
AuthorDate: Thu, 11 Jan 2018 19:47:47 -0500
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 17 Jan 2018 10:22:13 -0300

tools lib traceevent: Show contents (in hex) of data of unrecognized type records

When a record has an unrecognized type, an error message is reported,
but it would also be helpful to see the contents of that record. At
least show what it is in hex, instead of just showing a blank line.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Link: http://lkml.kernel.org/r/20180112004822.542204577@goodmis.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/traceevent/event-parse.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 344a034..e5f2acb 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -5566,8 +5566,14 @@ void pevent_print_event(struct pevent *pevent, struct trace_seq *s,
 
 	event = pevent_find_event_by_record(pevent, record);
 	if (!event) {
-		do_warning("ug! no event found for type %d",
-			   trace_parse_common_type(pevent, record->data));
+		int i;
+		int type = trace_parse_common_type(pevent, record->data);
+
+		do_warning("ug! no event found for type %d", type);
+		trace_seq_printf(s, "[UNKNOWN TYPE %d]", type);
+		for (i = 0; i < record->size; i++)
+			trace_seq_printf(s, " %02x",
+					 ((unsigned char *)record->data)[i]);
 		return;
 	}
 

[PATCH 07/10] lib traceevent: Use asprintf when possible

[Steven Rostedt]

[-- Attachment #1: 0007-lib-traceevent-Use-asprintf-when-possible.patch --]
[-- Type: text/plain, Size: 3884 bytes --]

From: Federico Vaga <federico.vaga@vaga.pv.it>

It makes the code clearer and less error prone.

clearer:
- less code
- the code is now using the same format to create strings dynamically

less error prone:
- no magic number +2 +9 +5 to compute the size
- no copy&paste of the strings to compute the size and to concatenate

The function `asprintf` is not POSIX standard but the program
was already using it. Later it can be decided to use only POSIX
functions, then we can easly replace all the `asprintf(3)` with a local
implementation of that function.

Link: http://lkml.kernel.org/r/20170802221558.9684-2-federico.vaga@vaga.pv.it

Signed-off-by: Federico Vaga <federico.vaga@vaga.pv.it>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 tools/lib/traceevent/event-plugin.c | 24 +++++++++---------------
 tools/lib/traceevent/parse-filter.c | 11 ++++-------
 2 files changed, 13 insertions(+), 22 deletions(-)

diff --git a/tools/lib/traceevent/event-plugin.c b/tools/lib/traceevent/event-plugin.c
index a16756ae3526..d542cb60ca1a 100644
--- a/tools/lib/traceevent/event-plugin.c
+++ b/tools/lib/traceevent/event-plugin.c
@@ -120,12 +120,12 @@ char **traceevent_plugin_list_options(void)
 		for (op = reg->options; op->name; op++) {
 			char *alias = op->plugin_alias ? op->plugin_alias : op->file;
 			char **temp = list;
+			int ret;
 
-			name = malloc(strlen(op->name) + strlen(alias) + 2);
-			if (!name)
+			ret = asprintf(&name, "%s:%s", alias, op->name);
+			if (ret < 0)
 				goto err;
 
-			sprintf(name, "%s:%s", alias, op->name);
 			list = realloc(list, count + 2);
 			if (!list) {
 				list = temp;
@@ -290,17 +290,14 @@ load_plugin(struct pevent *pevent, const char *path,
 	const char *alias;
 	char *plugin;
 	void *handle;
+	int ret;
 
-	plugin = malloc(strlen(path) + strlen(file) + 2);
-	if (!plugin) {
+	ret = asprintf(&plugin, "%s/%s", path, file);
+	if (ret < 0) {
 		warning("could not allocate plugin memory\n");
 		return;
 	}
 
-	strcpy(plugin, path);
-	strcat(plugin, "/");
-	strcat(plugin, file);
-
 	handle = dlopen(plugin, RTLD_NOW | RTLD_GLOBAL);
 	if (!handle) {
 		warning("could not load plugin '%s'\n%s\n",
@@ -391,6 +388,7 @@ load_plugins(struct pevent *pevent, const char *suffix,
 	char *home;
 	char *path;
 	char *envdir;
+	int ret;
 
 	if (pevent->flags & PEVENT_DISABLE_PLUGINS)
 		return;
@@ -421,16 +419,12 @@ load_plugins(struct pevent *pevent, const char *suffix,
 	if (!home)
 		return;
 
-	path = malloc(strlen(home) + strlen(LOCAL_PLUGIN_DIR) + 2);
-	if (!path) {
+	ret = asprintf(&path, "%s/%s", home, LOCAL_PLUGIN_DIR);
+	if (ret < 0) {
 		warning("could not allocate plugin memory\n");
 		return;
 	}
 
-	strcpy(path, home);
-	strcat(path, "/");
-	strcat(path, LOCAL_PLUGIN_DIR);
-
 	load_plugins_dir(pevent, suffix, path, load_plugin, data);
 
 	free(path);
diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c
index 315df0a70265..2410afdcbcfe 100644
--- a/tools/lib/traceevent/parse-filter.c
+++ b/tools/lib/traceevent/parse-filter.c
@@ -287,12 +287,10 @@ find_event(struct pevent *pevent, struct event_list **events,
 		sys_name = NULL;
 	}
 
-	reg = malloc(strlen(event_name) + 3);
-	if (reg == NULL)
+	ret = asprintf(&reg, "^%s$", event_name);
+	if (ret < 0)
 		return PEVENT_ERRNO__MEM_ALLOC_FAILED;
 
-	sprintf(reg, "^%s$", event_name);
-
 	ret = regcomp(&ereg, reg, REG_ICASE|REG_NOSUB);
 	free(reg);
 
@@ -300,13 +298,12 @@ find_event(struct pevent *pevent, struct event_list **events,
 		return PEVENT_ERRNO__INVALID_EVENT_NAME;
 
 	if (sys_name) {
-		reg = malloc(strlen(sys_name) + 3);
-		if (reg == NULL) {
+		ret = asprintf(&reg, "^%s$", sys_name);
+		if (ret < 0) {
 			regfree(&ereg);
 			return PEVENT_ERRNO__MEM_ALLOC_FAILED;
 		}
 
-		sprintf(reg, "^%s$", sys_name);
 		ret = regcomp(&sreg, reg, REG_ICASE|REG_NOSUB);
 		free(reg);
 		if (ret) {
-- 
2.13.2

[tip:perf/core] tools lib traceevent: Use asprintf when possible

[tip-bot for Federico Vaga]

Commit-ID:  67dfc376f3dfdc39b9125f32d5b24053a4da264f
Gitweb:     https://git.kernel.org/tip/67dfc376f3dfdc39b9125f32d5b24053a4da264f
Author:     Federico Vaga <federico.vaga@vaga.pv.it>
AuthorDate: Thu, 11 Jan 2018 19:47:48 -0500
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 17 Jan 2018 10:22:19 -0300

tools lib traceevent: Use asprintf when possible

It makes the code clearer and less error prone.

clearer:
- less code
- the code is now using the same format to create strings dynamically

less error prone:
- no magic number +2 +9 +5 to compute the size
- no copy&paste of the strings to compute the size and to concatenate

The function `asprintf` is not POSIX standard but the program
was already using it. Later it can be decided to use only POSIX
functions, then we can easly replace all the `asprintf(3)` with a local
implementation of that function.

Signed-off-by: Federico Vaga <federico.vaga@vaga.pv.it>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Federico Vaga <federico.vaga@vaga.pv.it>
Link: http://lkml.kernel.org/r/20170802221558.9684-2-federico.vaga@vaga.pv.it
Link: http://lkml.kernel.org/r/20180112004822.686281649@goodmis.org
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/traceevent/event-plugin.c | 24 +++++++++---------------
 tools/lib/traceevent/parse-filter.c | 11 ++++-------
 2 files changed, 13 insertions(+), 22 deletions(-)

diff --git a/tools/lib/traceevent/event-plugin.c b/tools/lib/traceevent/event-plugin.c
index a16756a..d542cb6 100644
--- a/tools/lib/traceevent/event-plugin.c
+++ b/tools/lib/traceevent/event-plugin.c
@@ -120,12 +120,12 @@ char **traceevent_plugin_list_options(void)
 		for (op = reg->options; op->name; op++) {
 			char *alias = op->plugin_alias ? op->plugin_alias : op->file;
 			char **temp = list;
+			int ret;
 
-			name = malloc(strlen(op->name) + strlen(alias) + 2);
-			if (!name)
+			ret = asprintf(&name, "%s:%s", alias, op->name);
+			if (ret < 0)
 				goto err;
 
-			sprintf(name, "%s:%s", alias, op->name);
 			list = realloc(list, count + 2);
 			if (!list) {
 				list = temp;
@@ -290,17 +290,14 @@ load_plugin(struct pevent *pevent, const char *path,
 	const char *alias;
 	char *plugin;
 	void *handle;
+	int ret;
 
-	plugin = malloc(strlen(path) + strlen(file) + 2);
-	if (!plugin) {
+	ret = asprintf(&plugin, "%s/%s", path, file);
+	if (ret < 0) {
 		warning("could not allocate plugin memory\n");
 		return;
 	}
 
-	strcpy(plugin, path);
-	strcat(plugin, "/");
-	strcat(plugin, file);
-
 	handle = dlopen(plugin, RTLD_NOW | RTLD_GLOBAL);
 	if (!handle) {
 		warning("could not load plugin '%s'\n%s\n",
@@ -391,6 +388,7 @@ load_plugins(struct pevent *pevent, const char *suffix,
 	char *home;
 	char *path;
 	char *envdir;
+	int ret;
 
 	if (pevent->flags & PEVENT_DISABLE_PLUGINS)
 		return;
@@ -421,16 +419,12 @@ load_plugins(struct pevent *pevent, const char *suffix,
 	if (!home)
 		return;
 
-	path = malloc(strlen(home) + strlen(LOCAL_PLUGIN_DIR) + 2);
-	if (!path) {
+	ret = asprintf(&path, "%s/%s", home, LOCAL_PLUGIN_DIR);
+	if (ret < 0) {
 		warning("could not allocate plugin memory\n");
 		return;
 	}
 
-	strcpy(path, home);
-	strcat(path, "/");
-	strcat(path, LOCAL_PLUGIN_DIR);
-
 	load_plugins_dir(pevent, suffix, path, load_plugin, data);
 
 	free(path);
diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c
index 315df0a..2410afd 100644
--- a/tools/lib/traceevent/parse-filter.c
+++ b/tools/lib/traceevent/parse-filter.c
@@ -287,12 +287,10 @@ find_event(struct pevent *pevent, struct event_list **events,
 		sys_name = NULL;
 	}
 
-	reg = malloc(strlen(event_name) + 3);
-	if (reg == NULL)
+	ret = asprintf(&reg, "^%s$", event_name);
+	if (ret < 0)
 		return PEVENT_ERRNO__MEM_ALLOC_FAILED;
 
-	sprintf(reg, "^%s$", event_name);
-
 	ret = regcomp(&ereg, reg, REG_ICASE|REG_NOSUB);
 	free(reg);
 
@@ -300,13 +298,12 @@ find_event(struct pevent *pevent, struct event_list **events,
 		return PEVENT_ERRNO__INVALID_EVENT_NAME;
 
 	if (sys_name) {
-		reg = malloc(strlen(sys_name) + 3);
-		if (reg == NULL) {
+		ret = asprintf(&reg, "^%s$", sys_name);
+		if (ret < 0) {
 			regfree(&ereg);
 			return PEVENT_ERRNO__MEM_ALLOC_FAILED;
 		}
 
-		sprintf(reg, "^%s$", sys_name);
 		ret = regcomp(&sreg, reg, REG_ICASE|REG_NOSUB);
 		free(reg);
 		if (ret) {

[PATCH 08/10] lib traceevent: Add UL suffix to MISSING_EVENTS

[Steven Rostedt]

[-- Attachment #1: 0008-lib-traceevent-Add-UL-suffix-to-MISSING_EVENTS.patch --]
[-- Type: text/plain, Size: 869 bytes --]

From: Michael Sartain <mikesart@fastmail.com>

Add UL suffix to MISSING_EVENTS since ints shouldn't be left shifted by 31.

Link: http://lkml.kernel.org/r/20171016165542.13038-4-mikesart@fastmail.com

Signed-off-by: Michael Sartain <mikesart@fastmail.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 tools/lib/traceevent/kbuffer-parse.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/lib/traceevent/kbuffer-parse.c b/tools/lib/traceevent/kbuffer-parse.c
index c94e3641b046..ca424b157e46 100644
--- a/tools/lib/traceevent/kbuffer-parse.c
+++ b/tools/lib/traceevent/kbuffer-parse.c
@@ -24,8 +24,8 @@
 
 #include "kbuffer.h"
 
-#define MISSING_EVENTS (1 << 31)
-#define MISSING_STORED (1 << 30)
+#define MISSING_EVENTS (1UL << 31)
+#define MISSING_STORED (1UL << 30)
 
 #define COMMIT_MASK ((1 << 27) - 1)
 
-- 
2.13.2

[tip:perf/core] tools lib traceevent: Add UL suffix to MISSING_EVENTS

[tip-bot for Michael Sartain]

Commit-ID:  6d36ce261614fbac3557cc58ba6a33424944c8a2
Gitweb:     https://git.kernel.org/tip/6d36ce261614fbac3557cc58ba6a33424944c8a2
Author:     Michael Sartain <mikesart@fastmail.com>
AuthorDate: Thu, 11 Jan 2018 19:47:49 -0500
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 17 Jan 2018 10:22:49 -0300

tools lib traceevent: Add UL suffix to MISSING_EVENTS

Add UL suffix to MISSING_EVENTS since ints shouldn't be left shifted by 31.

Signed-off-by: Michael Sartain <mikesart@fastmail.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Link: http://lkml.kernel.org/r/20171016165542.13038-4-mikesart@fastmail.com
Link: http://lkml.kernel.org/r/20180112004822.829533885@goodmis.org
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/traceevent/kbuffer-parse.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/lib/traceevent/kbuffer-parse.c b/tools/lib/traceevent/kbuffer-parse.c
index c94e364..ca424b1 100644
--- a/tools/lib/traceevent/kbuffer-parse.c
+++ b/tools/lib/traceevent/kbuffer-parse.c
@@ -24,8 +24,8 @@
 
 #include "kbuffer.h"
 
-#define MISSING_EVENTS (1 << 31)
-#define MISSING_STORED (1 << 30)
+#define MISSING_EVENTS (1UL << 31)
+#define MISSING_STORED (1UL << 30)
 
 #define COMMIT_MASK ((1 << 27) - 1)
 

[PATCH 09/10] lib traceeevent: Fix missing break in FALSE case of pevent_filter_clear_trivial()

[Steven Rostedt]

[-- Attachment #1: 0009-lib-traceeevent-Fix-missing-break-in-FALSE-case-of-p.patch --]
[-- Type: text/plain, Size: 1126 bytes --]

From: Taeung Song <treeze.taeung@gmail.com>

Currently the FILTER_TRIVIAL_FALSE case has a missing break statement, if
the trivial type is FALSE, it will also run into the TRUE case, and always
be skipped as the TRUE statement will continue the loop on the inverse
condition of the FALSE statement.

Link: http://lkml.kernel.org/r/1493218540-12296-1-git-send-email-treeze.taeung@gmail.com

Reported-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 tools/lib/traceevent/parse-filter.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c
index 2410afdcbcfe..2b9048f90bae 100644
--- a/tools/lib/traceevent/parse-filter.c
+++ b/tools/lib/traceevent/parse-filter.c
@@ -1631,6 +1631,7 @@ int pevent_filter_clear_trivial(struct event_filter *filter,
 		case FILTER_TRIVIAL_FALSE:
 			if (filter_type->filter->boolean.value)
 				continue;
+			break;
 		case FILTER_TRIVIAL_TRUE:
 			if (!filter_type->filter->boolean.value)
 				continue;
-- 
2.13.2

[PATCH] parse-events: Fix the FALSE case in pevent_filter_clear_trivial()

[Taeung Song]

Currently the FILTER_TRIVIAL_FALSE hasn't break statement
so if the trivial type is FALSE, it'll be hit always.

So add break statement at the FALSE case
to correctly remove trivial filters.

Reported-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 parse-filter.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/parse-filter.c b/parse-filter.c
index 7c214ce..c2fd26f 100644
--- a/parse-filter.c
+++ b/parse-filter.c
@@ -1634,6 +1634,7 @@ int pevent_filter_clear_trivial(struct event_filter *filter,
 		case FILTER_TRIVIAL_FALSE:
 			if (filter_type->filter->boolean.value)
 				continue;
+			break;
 		case FILTER_TRIVIAL_TRUE:
 			if (!filter_type->filter->boolean.value)
 				continue;
-- 
2.7.4

[tip:perf/core] tools lib traceevent: Fix missing break in FALSE case of pevent_filter_clear_trivial()

[tip-bot for Taeung Song]

Commit-ID:  806efaed3cacab1521895d20bb3b5ed610909299
Gitweb:     https://git.kernel.org/tip/806efaed3cacab1521895d20bb3b5ed610909299
Author:     Taeung Song <treeze.taeung@gmail.com>
AuthorDate: Thu, 11 Jan 2018 19:47:50 -0500
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 17 Jan 2018 10:22:57 -0300

tools lib traceevent: Fix missing break in FALSE case of pevent_filter_clear_trivial()

Currently the FILTER_TRIVIAL_FALSE case has a missing break statement,
if the trivial type is FALSE, it will also run into the TRUE case, and
always be skipped as the TRUE statement will continue the loop on the
inverse condition of the FALSE statement.

Reported-by: Namhyung Kim <namhyung@kernel.org>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Link: http://lkml.kernel.org/r/20180112004823.012918807@goodmis.org
Link: http://lkml.kernel.org/r/1493218540-12296-1-git-send-email-treeze.taeung@gmail.com
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/traceevent/parse-filter.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c
index 2410afd..2b9048f 100644
--- a/tools/lib/traceevent/parse-filter.c
+++ b/tools/lib/traceevent/parse-filter.c
@@ -1631,6 +1631,7 @@ int pevent_filter_clear_trivial(struct event_filter *filter,
 		case FILTER_TRIVIAL_FALSE:
 			if (filter_type->filter->boolean.value)
 				continue;
+			break;
 		case FILTER_TRIVIAL_TRUE:
 			if (!filter_type->filter->boolean.value)
 				continue;

Re: [PATCH 09/10] lib traceeevent: Fix missing break in FALSE case of pevent_filter_clear_trivial()

[Taeung Song]

Hi Steven,

I found a trivial typo "eee" on the commit log title
It seems better to change "lib traceeevent" to " lib traceevent",
if you want to do it..

Thanks,
Taeung

On 01/12/2018 09:47 AM, Steven Rostedt wrote:

Re: [PATCH 09/10] lib traceeevent: Fix missing break in FALSE case of pevent_filter_clear_trivial()

[Steven Rostedt]

On Fri, 12 Jan 2018 10:00:34 +0900
Taeung Song <treeze.taeung@gmail.com> wrote:

> Hi Steven,
> 
> I found a trivial typo "eee" on the commit log title

That traceevent got too close to a nuclear power plant.

> It seems better to change "lib traceeevent" to " lib traceevent",
> if you want to do it..

Yeah it should be changed. Not sure I want to send a full series for
such an update. I can send an updated patch, another full
series, or Arnaldo could manually fix it.

Arnaldo, your call.

-- Steve

Re: [PATCH 09/10] lib traceeevent: Fix missing break in FALSE case of pevent_filter_clear_trivial()

[Arnaldo Carvalho de Melo]

Em Thu, Jan 11, 2018 at 08:14:35PM -0500, Steven Rostedt escreveu:
> On Fri, 12 Jan 2018 10:00:34 +0900
> Taeung Song <treeze.taeung@gmail.com> wrote:
> 
> > Hi Steven,
> > 
> > I found a trivial typo "eee" on the commit log title
> 
> That traceevent got too close to a nuclear power plant.
> 
> > It seems better to change "lib traceeevent" to " lib traceevent",
> > if you want to do it..
> 
> Yeah it should be changed. Not sure I want to send a full series for
> such an update. I can send an updated patch, another full
> series, or Arnaldo could manually fix it.
> 
> Arnaldo, your call.

I'll fix it, thanks.

- Arnaldo

[PATCH 10/10] lib traceevent: Fix get_field_str() for dynamic strings

[Steven Rostedt]

[-- Attachment #1: 0010-lib-traceevent-Fix-get_field_str-for-dynamic-strings.patch --]
[-- Type: text/plain, Size: 1607 bytes --]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

If a field is a dynamic string, get_field_str() returned just the
offset/size value and not the string. Have it parse the offset/size
correctly to return the actual string. Otherwise filtering fails when trying
to filter fields that are dynamic strings.

Reported-by: gopanapalli pradeep <prap_hai@yahoo.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 tools/lib/traceevent/parse-filter.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c
index 2b9048f90bae..431e8b309f6e 100644
--- a/tools/lib/traceevent/parse-filter.c
+++ b/tools/lib/traceevent/parse-filter.c
@@ -1877,17 +1877,25 @@ static const char *get_field_str(struct filter_arg *arg, struct pevent_record *r
 	struct pevent *pevent;
 	unsigned long long addr;
 	const char *val = NULL;
+	unsigned int size;
 	char hex[64];
 
 	/* If the field is not a string convert it */
 	if (arg->str.field->flags & FIELD_IS_STRING) {
 		val = record->data + arg->str.field->offset;
+		size = arg->str.field->size;
+
+		if (arg->str.field->flags & FIELD_IS_DYNAMIC) {
+			addr = *(unsigned int *)val;
+			val = record->data + (addr & 0xffff);
+			size = addr >> 16;
+		}
 
 		/*
 		 * We need to copy the data since we can't be sure the field
 		 * is null terminated.
 		 */
-		if (*(val + arg->str.field->size - 1)) {
+		if (*(val + size - 1)) {
 			/* copy it */
 			memcpy(arg->str.buffer, val, arg->str.field->size);
 			/* the buffer is already NULL terminated */
-- 
2.13.2

[tip:perf/core] tools lib traceevent: Fix get_field_str() for dynamic strings

[tip-bot for Steven Rostedt (VMware)]

Commit-ID:  d777f8de99b05d399c0e4e51cdce016f26bd971b
Gitweb:     https://git.kernel.org/tip/d777f8de99b05d399c0e4e51cdce016f26bd971b
Author:     Steven Rostedt (VMware) <rostedt@goodmis.org>
AuthorDate: Thu, 11 Jan 2018 19:47:51 -0500
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 17 Jan 2018 10:23:21 -0300

tools lib traceevent: Fix get_field_str() for dynamic strings

If a field is a dynamic string, get_field_str() returned just the
offset/size value and not the string. Have it parse the offset/size
correctly to return the actual string. Otherwise filtering fails when
trying to filter fields that are dynamic strings.

Reported-by: Gopanapalli Pradeep <prap_hai@yahoo.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Link: http://lkml.kernel.org/r/20180112004823.146333275@goodmis.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/traceevent/parse-filter.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c
index 2b9048f..431e8b3 100644
--- a/tools/lib/traceevent/parse-filter.c
+++ b/tools/lib/traceevent/parse-filter.c
@@ -1877,17 +1877,25 @@ static const char *get_field_str(struct filter_arg *arg, struct pevent_record *r
 	struct pevent *pevent;
 	unsigned long long addr;
 	const char *val = NULL;
+	unsigned int size;
 	char hex[64];
 
 	/* If the field is not a string convert it */
 	if (arg->str.field->flags & FIELD_IS_STRING) {
 		val = record->data + arg->str.field->offset;
+		size = arg->str.field->size;
+
+		if (arg->str.field->flags & FIELD_IS_DYNAMIC) {
+			addr = *(unsigned int *)val;
+			val = record->data + (addr & 0xffff);
+			size = addr >> 16;
+		}
 
 		/*
 		 * We need to copy the data since we can't be sure the field
 		 * is null terminated.
 		 */
-		if (*(val + arg->str.field->size - 1)) {
+		if (*(val + size - 1)) {
 			/* copy it */
 			memcpy(arg->str.buffer, val, arg->str.field->size);
 			/* the buffer is already NULL terminated */

Re: [PATCH 00/10] tools lib traceveent: Pull updates from trace-cmd library

[Namhyung Kim]

Hi Steve,

On Thu, Jan 11, 2018 at 07:47:41PM -0500, Steven Rostedt wrote:
> 
> I went through the commits that were added to event-parse in trace-cmd
> and synced them up for what is in tools/lib/traceevent. These are
> patches that are missing from the kernel tree.

If I'm not too late..

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung


> 
> Federico Vaga (1):
>       lib traceevent: Use asprintf when possible
> 
> Jan Kiszka (1):
>       lib traceevent: Print value of unknown symbolic fields
> 
> Michael Sartain (2):
>       lib, traceevent: Fix bad force_token escape sequence
>       lib traceevent: Add UL suffix to MISSING_EVENTS
> 
> Steven Rostedt (VMware) (5):
>       lib traceevent: Show value of flags that have not been parsed
>       lib traceevent: Simplify pointer print logic and fix %pF
>       lib traceevent: Handle new pointer processing of bprint strings
>       lib traceevent: Show contents (in hex) of data of unrecognized type records
>       lib traceevent: Fix get_field_str() for dynamic strings
> 
> Taeung Song (1):
>       lib traceeevent: Fix missing break in FALSE case of pevent_filter_clear_trivial()
> 
> ----
>  tools/lib/traceevent/event-parse.c   | 62 +++++++++++++++++++++++++++++-------
>  tools/lib/traceevent/event-plugin.c  | 24 ++++++--------
>  tools/lib/traceevent/kbuffer-parse.c |  4 +--
>  tools/lib/traceevent/parse-filter.c  | 22 ++++++++-----
>  4 files changed, 76 insertions(+), 36 deletions(-)

[PATCH][trace-cmd] Print value of unknown symbolic fields

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

Aligns trace-cmd with the behavior of the kernel.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 event-parse.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/event-parse.c b/event-parse.c
index 606da5b..25e0874 100644
--- a/event-parse.c
+++ b/event-parse.c
@@ -3960,6 +3960,8 @@ static void print_str_arg(struct trace_seq *s, void *data, int size,
 				break;
 			}
 		}
+		if (!flag)
+			trace_seq_printf(s, "0x%llx", val);
 		break;
 	case PRINT_HEX:
 		if (arg->hex.field->type == PRINT_DYNAMIC_ARRAY) {
-- 
2.12.3

Re: [PATCH][trace-cmd] Print value of unknown symbolic fields

[Steven Rostedt]

On Thu, 28 Sep 2017 22:13:10 +0200
Jan Kiszka <jan.kiszka@siemens.com> wrote:

> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Aligns trace-cmd with the behavior of the kernel.
> 

Thanks, applied.

-- Steve

> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  event-parse.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/event-parse.c b/event-parse.c
> index 606da5b..25e0874 100644
> --- a/event-parse.c
> +++ b/event-parse.c
> @@ -3960,6 +3960,8 @@ static void print_str_arg(struct trace_seq *s, void *data, int size,
>  				break;
>  			}
>  		}
> +		if (!flag)
> +			trace_seq_printf(s, "0x%llx", val);
>  		break;
>  	case PRINT_HEX:
>  		if (arg->hex.field->type == PRINT_DYNAMIC_ARRAY) {

Re: [PATCH][trace-cmd] Print value of unknown symbolic fields

[Steven Rostedt]

On Thu, 28 Sep 2017 22:13:10 +0200
Jan Kiszka <jan.kiszka@siemens.com> wrote:

> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Aligns trace-cmd with the behavior of the kernel.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  event-parse.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/event-parse.c b/event-parse.c
> index 606da5b..25e0874 100644
> --- a/event-parse.c
> +++ b/event-parse.c
> @@ -3960,6 +3960,8 @@ static void print_str_arg(struct trace_seq *s, void *data, int size,
>  				break;
>  			}
>  		}
> +		if (!flag)
> +			trace_seq_printf(s, "0x%llx", val);
>  		break;
>  	case PRINT_HEX:
>  		if (arg->hex.field->type == PRINT_DYNAMIC_ARRAY) {

I'm going to also add this patch, to do the same for flags:

diff --git a/event-parse.c b/event-parse.c
index 25e0874..7ef66f8 100644
--- a/event-parse.c
+++ b/event-parse.c
@@ -3950,6 +3950,11 @@ static void print_str_arg(struct trace_seq *s, void *data, int size,
 				val &= ~fval;
 			}
 		}
+		if (val) {
+			if (print && arg->flags.delim)
+				trace_seq_puts(s, arg->flags.delim);
+			trace_seq_printf(s, "0x%llx", val);
+		}
 		break;
 	case PRINT_SYMBOL:
 		val = eval_num_arg(data, size, event, arg->symbol.field);


-- Steve

[PATCH V3 1/2] use direname instead of custom code

[Federico Vaga]

Prefer well known functions like `dirname(3)` instead of custom
implementation for the same functionality

Signed-off-by: Federico Vaga <federico.vaga@vaga.pv.it>
---
 trace-record.c | 45 ++++++++++++++++++++++++++++++---------------
 1 file changed, 30 insertions(+), 15 deletions(-)

diff --git a/trace-record.c b/trace-record.c
index 020a373..79e4fe4 100644
--- a/trace-record.c
+++ b/trace-record.c
@@ -45,6 +45,7 @@
 #include <glob.h>
 #include <errno.h>
 #include <limits.h>
+#include <libgen.h>
 
 #include "trace-local.h"
 #include "trace-msg.h"
@@ -2215,12 +2216,24 @@ static void set_max_graph_depth(struct buffer_instance *instance, char *max_grap
 		die("could not write to max_graph_depth");
 }
 
+
+/**
+ * create_event - create and event descriptor
+ * @instance: instance to use
+ * @path: path to event attribute
+ * @old_event: event descriptor to use as base
+ *
+ * NOTE: the function purpose is to create a data structure to describe
+ * an ftrace event. During the process it becomes handy to change the
+ * string `path`. So, do not rely on the content of `path` after you
+ * invoke this function.
+ */
 static struct event_list *
 create_event(struct buffer_instance *instance, char *path, struct event_list *old_event)
 {
 	struct event_list *event;
 	struct stat st;
-	char *p;
+	char *p, *path_dirname;
 	int ret;
 
 	event = malloc(sizeof(*event));
@@ -2234,14 +2247,14 @@ create_event(struct buffer_instance *instance, char *path, struct event_list *ol
 		if (!event->filter_file)
 			die("malloc filter file");
 	}
-	for (p = path + strlen(path) - 1; p > path; p--)
-		if (*p == '/')
-			break;
-	*p = '\0';
-	p = malloc(strlen(path) + strlen("/enable") + 1);
+
+	path_dirname = dirname(path);
+
+	p = malloc(strlen(path_dirname) + strlen("/enable") + 1);
 	if (!p)
 		die("Failed to allocate enable path for %s", path);
-	sprintf(p, "%s/enable", path);
+	sprintf(p, "%s/enable", path_dirname);
+
 	ret = stat(p, &st);
 	if (ret >= 0)
 		event->enable_file = p;
@@ -2249,10 +2262,11 @@ create_event(struct buffer_instance *instance, char *path, struct event_list *ol
 		free(p);
 
 	if (event->trigger) {
-		p = malloc(strlen(path) + strlen("/trigger") + 1);
+		p = malloc(strlen(path_dirname) + strlen("/trigger") + 1);
 		if (!p)
 			die("Failed to allocate trigger path for %s", path);
-		sprintf(p, "%s/trigger", path);
+		sprintf(p, "%s/trigger", path_dirname);
+
 		ret = stat(p, &st);
 		if (ret > 0)
 			die("trigger specified but not supported by this kernel");
@@ -2266,8 +2280,7 @@ static void make_sched_event(struct buffer_instance *instance,
 			     struct event_list **event, struct event_list *sched,
 			     const char *sched_path)
 {
-	char *path;
-	char *p;
+	char *path, *path_dirname, *sched_filter_file_tmp;
 
 	/* Do nothing if the event already exists */
 	if (*event)
@@ -2277,11 +2290,13 @@ static void make_sched_event(struct buffer_instance *instance,
 	if (!path)
 		die("Failed to allocate path for %s", sched_path);
 
-	sprintf(path, "%s", sched->filter_file);
+	/* we do not want to corrupt sched->filter_file when using dirname() */
+	sched_filter_file_tmp = strdup(sched->filter_file);
+	if (!sched_filter_file_tmp)
+		die("Failed to allocate path for %s", sched_path);
+	path_dirname = dirname(sched_filter_file_tmp);
 
-	/* Remove the /filter from filter file */
-	p = path + strlen(path) - strlen("filter");
-	sprintf(p, "%s/filter", sched_path);
+	sprintf(path, "%s/%s/filter", path_dirname, sched_path);
 
 	*event = create_event(instance, path, sched);
 	free(path);
-- 
2.13.3

[PATCH V3 2/2] It makes the code clearer and less error prone.

[Federico Vaga]

clearer:
- less code
- the code is now using the same format to create strings dynamically

less error prone:
- no magic number +2 +9 +5 to compute the size
- no copy&paste of the strings to compute the size and to concatenate

The function `asprintf` is not POSIX standard but the program
was already using it. Later it can be decided to use only POSIX
functions, then we can easly replace all the `asprintf(3)` with a local
implementation of that function.

Signed-off-by: Federico Vaga <federico.vaga@vaga.pv.it>
---
 event-plugin.c   | 24 ++++++++--------------
 parse-filter.c   | 11 ++++------
 trace-list.c     |  8 ++++----
 trace-output.c   |  6 +++---
 trace-record.c   | 62 ++++++++++++++++++++++++--------------------------------
 trace-recorder.c | 11 +++++-----
 trace-show.c     |  8 ++++----
 trace-split.c    |  7 ++++---
 trace-stat.c     |  7 ++++---
 trace-util.c     | 62 ++++++++++++++++++++++++--------------------------------
 10 files changed, 89 insertions(+), 117 deletions(-)

diff --git a/event-plugin.c b/event-plugin.c
index a16756a..d542cb6 100644
--- a/event-plugin.c
+++ b/event-plugin.c
@@ -120,12 +120,12 @@ char **traceevent_plugin_list_options(void)
 		for (op = reg->options; op->name; op++) {
 			char *alias = op->plugin_alias ? op->plugin_alias : op->file;
 			char **temp = list;
+			int ret;
 
-			name = malloc(strlen(op->name) + strlen(alias) + 2);
-			if (!name)
+			ret = asprintf(&name, "%s:%s", alias, op->name);
+			if (ret < 0)
 				goto err;
 
-			sprintf(name, "%s:%s", alias, op->name);
 			list = realloc(list, count + 2);
 			if (!list) {
 				list = temp;
@@ -290,17 +290,14 @@ load_plugin(struct pevent *pevent, const char *path,
 	const char *alias;
 	char *plugin;
 	void *handle;
+	int ret;
 
-	plugin = malloc(strlen(path) + strlen(file) + 2);
-	if (!plugin) {
+	ret = asprintf(&plugin, "%s/%s", path, file);
+	if (ret < 0) {
 		warning("could not allocate plugin memory\n");
 		return;
 	}
 
-	strcpy(plugin, path);
-	strcat(plugin, "/");
-	strcat(plugin, file);
-
 	handle = dlopen(plugin, RTLD_NOW | RTLD_GLOBAL);
 	if (!handle) {
 		warning("could not load plugin '%s'\n%s\n",
@@ -391,6 +388,7 @@ load_plugins(struct pevent *pevent, const char *suffix,
 	char *home;
 	char *path;
 	char *envdir;
+	int ret;
 
 	if (pevent->flags & PEVENT_DISABLE_PLUGINS)
 		return;
@@ -421,16 +419,12 @@ load_plugins(struct pevent *pevent, const char *suffix,
 	if (!home)
 		return;
 
-	path = malloc(strlen(home) + strlen(LOCAL_PLUGIN_DIR) + 2);
-	if (!path) {
+	ret = asprintf(&path, "%s/%s", home, LOCAL_PLUGIN_DIR);
+	if (ret < 0) {
 		warning("could not allocate plugin memory\n");
 		return;
 	}
 
-	strcpy(path, home);
-	strcat(path, "/");
-	strcat(path, LOCAL_PLUGIN_DIR);
-
 	load_plugins_dir(pevent, suffix, path, load_plugin, data);
 
 	free(path);
diff --git a/parse-filter.c b/parse-filter.c
index 8e302c4..ebf2a95 100644
--- a/parse-filter.c
+++ b/parse-filter.c
@@ -287,12 +287,10 @@ find_event(struct pevent *pevent, struct event_list **events,
 		sys_name = NULL;
 	}
 
-	reg = malloc(strlen(event_name) + 3);
-	if (reg == NULL)
+	ret = asprintf(&reg, "^%s$", event_name);
+	if (ret < 0)
 		return PEVENT_ERRNO__MEM_ALLOC_FAILED;
 
-	sprintf(reg, "^%s$", event_name);
-
 	ret = regcomp(&ereg, reg, REG_ICASE|REG_NOSUB);
 	free(reg);
 
@@ -300,13 +298,12 @@ find_event(struct pevent *pevent, struct event_list **events,
 		return PEVENT_ERRNO__INVALID_EVENT_NAME;
 
 	if (sys_name) {
-		reg = malloc(strlen(sys_name) + 3);
-		if (reg == NULL) {
+		ret = asprintf(&reg, "^%s$", sys_name);
+		if (ret < 0) {
 			regfree(&ereg);
 			return PEVENT_ERRNO__MEM_ALLOC_FAILED;
 		}
 
-		sprintf(reg, "^%s$", sys_name);
 		ret = regcomp(&sreg, reg, REG_ICASE|REG_NOSUB);
 		free(reg);
 		if (ret) {
diff --git a/trace-list.c b/trace-list.c
index 293635f..32b19bc 100644
--- a/trace-list.c
+++ b/trace-list.c
@@ -133,6 +133,7 @@ static char *get_event_file(const char *type, char *buf, int len)
 	char *event;
 	char *path;
 	char *file;
+	int ret;
 
 	if (buf[len-1] == '\n')
 		buf[len-1] = '\0';
@@ -146,11 +147,10 @@ static char *get_event_file(const char *type, char *buf, int len)
 		die("no event found in %s\n", buf);
 
 	path = tracecmd_get_tracing_file("events");
-	file = malloc(strlen(path) + strlen(system) + strlen(event) +
-		      strlen(type) + strlen("///") + 1);
-	if (!file)
+	ret = asprintf(&file, "%s/%s/%s/%s", path, system, event, type);
+	if (ret < 0)
 		die("Failed to allocate event file %s %s", system, event);
-	sprintf(file, "%s/%s/%s/%s", path, system, event, type);
+
 	tracecmd_put_tracing_file(path);
 
 	return file;
diff --git a/trace-output.c b/trace-output.c
index bfe6331..26ed560 100644
--- a/trace-output.c
+++ b/trace-output.c
@@ -234,16 +234,16 @@ static char *get_tracing_file(struct tracecmd_output *handle, const char *name)
 {
 	const char *tracing;
 	char *file;
+	int ret;
 
 	tracing = find_tracing_dir(handle);
 	if (!tracing)
 		return NULL;
 
-	file = malloc(strlen(tracing) + strlen(name) + 2);
-	if (!file)
+	ret = asprintf(&file, "%s/%s", tracing, name);
+	if (ret < 0)
 		return NULL;
 
-	sprintf(file, "%s/%s", tracing, name);
 	return file;
 }
 
diff --git a/trace-record.c b/trace-record.c
index 79e4fe4..b1993c7 100644
--- a/trace-record.c
+++ b/trace-record.c
@@ -724,14 +724,12 @@ get_instance_file(struct buffer_instance *instance, const char *file)
 {
 	char *buf;
 	char *path;
+	int ret;
 
 	if (instance->name) {
-		buf = malloc(strlen(instance->name) +
-			     strlen(file) + strlen("instances//") + 1);
-		if (!buf)
+		ret = asprintf(&buf, "instances/%s/%s", instance->name, file);
+		if (ret < 0)
 			die("Failed to allocate name for %s/%s", instance->name, file);
-		sprintf(buf, "instances/%s/%s", instance->name, file);
-
 		path = tracecmd_get_tracing_file(buf);
 		free(buf);
 	} else
@@ -745,17 +743,15 @@ get_instance_dir(struct buffer_instance *instance)
 {
 	char *buf;
 	char *path;
+	int ret;
 
 	/* only works for instances */
 	if (!instance->name)
 		return NULL;
 
-	buf = malloc(strlen(instance->name) +
-		     strlen("instances/") + 1);
-	if (!buf)
+	ret = asprintf(&buf, "instances/%s", instance->name);
+	if (ret < 0)
 		die("Failed to allocate for instance %s", instance->name);
-	sprintf(buf, "instances/%s", instance->name);
-
 	path = tracecmd_get_tracing_file(buf);
 	free(buf);
 
@@ -2250,11 +2246,9 @@ create_event(struct buffer_instance *instance, char *path, struct event_list *ol
 
 	path_dirname = dirname(path);
 
-	p = malloc(strlen(path_dirname) + strlen("/enable") + 1);
-	if (!p)
-		die("Failed to allocate enable path for %s", path);
-	sprintf(p, "%s/enable", path_dirname);
-
+	ret = asprintf(&p, "%s/enable", path_dirname);
+	if (ret < 0)
+		die("Failed to allocate enable path for %s", path_dirname);
 	ret = stat(p, &st);
 	if (ret >= 0)
 		event->enable_file = p;
@@ -2262,11 +2256,9 @@ create_event(struct buffer_instance *instance, char *path, struct event_list *ol
 		free(p);
 
 	if (event->trigger) {
-		p = malloc(strlen(path_dirname) + strlen("/trigger") + 1);
-		if (!p)
-			die("Failed to allocate trigger path for %s", path);
-		sprintf(p, "%s/trigger", path_dirname);
-
+		ret = asprintf(&p, "%s/trigger", path_dirname);
+		if (ret < 0)
+			die("Failed to allocate trigger path for %s", path_dirname);
 		ret = stat(p, &st);
 		if (ret > 0)
 			die("trigger specified but not supported by this kernel");
@@ -2281,22 +2273,22 @@ static void make_sched_event(struct buffer_instance *instance,
 			     const char *sched_path)
 {
 	char *path, *path_dirname, *sched_filter_file_tmp;
+	int ret;
 
 	/* Do nothing if the event already exists */
 	if (*event)
 		return;
 
-	path = malloc(strlen(sched->filter_file) + strlen(sched_path) + 2);
-	if (!path)
-		die("Failed to allocate path for %s", sched_path);
-
 	/* we do not want to corrupt sched->filter_file when using dirname() */
 	sched_filter_file_tmp = strdup(sched->filter_file);
 	if (!sched_filter_file_tmp)
 		die("Failed to allocate path for %s", sched_path);
 	path_dirname = dirname(sched_filter_file_tmp);
 
-	sprintf(path, "%s/%s/filter", path_dirname, sched_path);
+	ret = asprintf(&path, "%s/%s/filter", path_dirname, sched_path);
+	free(sched_filter_file_tmp);
+	if (ret < 0)
+		die("Failed to allocate path for %s", sched_path);
 
 	*event = create_event(instance, path, sched);
 	free(path);
@@ -2325,10 +2317,9 @@ static int expand_event_files(struct buffer_instance *instance,
 	int ret;
 	int i;
 
-	p = malloc(strlen(file) + strlen("events//filter") + 1);
-	if (!p)
+	ret = asprintf(&p, "events/%s/filter", file);
+	if (ret < 0)
 		die("Failed to allocate event filter path for %s", file);
-	sprintf(p, "events/%s/filter", file);
 
 	path = get_instance_file(instance, p);
 
@@ -3984,6 +3975,8 @@ static int recording_all_events(void)
 
 static void add_trigger(struct event_list *event, const char *trigger)
 {
+	int ret;
+
 	if (event->trigger) {
 		event->trigger = realloc(event->trigger,
 					 strlen(event->trigger) + strlen("\n") +
@@ -3991,10 +3984,9 @@ static void add_trigger(struct event_list *event, const char *trigger)
 		strcat(event->trigger, "\n");
 		strcat(event->trigger, trigger);
 	} else {
-		event->trigger = malloc(strlen(trigger) + 1);
-		if (!event->trigger)
+		ret = asprintf(&event->trigger, "%s", trigger);
+		if (ret < 0)
 			die("Failed to allocate event trigger");
-		sprintf(event->trigger, "%s", trigger);
 	}
 }
 
@@ -4389,6 +4381,7 @@ void trace_record (int argc, char **argv)
 
 	for (;;) {
 		int option_index = 0;
+		int ret;
 		const char *opts;
 		static struct option long_options[] = {
 			{"date", no_argument, NULL, OPT_date},
@@ -4453,12 +4446,9 @@ void trace_record (int argc, char **argv)
 				strcat(last_event->filter, optarg);
 				strcat(last_event->filter, ")");
 			} else {
-				last_event->filter =
-					malloc(strlen(optarg) +
-					       strlen("()") + 1);
-				if (!last_event->filter)
+				ret = asprintf(&last_event->filter, "(%s)", optarg);
+				if (ret < 0)
 					die("Failed to allocate filter %s", optarg);
-				sprintf(last_event->filter, "(%s)", optarg);
 			}
 			break;
 
diff --git a/trace-recorder.c b/trace-recorder.c
index 1b9d364..85150fd 100644
--- a/trace-recorder.c
+++ b/trace-recorder.c
@@ -156,14 +156,13 @@ tracecmd_create_buffer_recorder_fd2(int fd, int fd2, int cpu, unsigned flags,
 	recorder->fd1 = fd;
 	recorder->fd2 = fd2;
 
-	path = malloc(strlen(buffer) + 40);
-	if (!path)
-		goto out_free;
-
 	if (flags & TRACECMD_RECORD_SNAPSHOT)
-		sprintf(path, "%s/per_cpu/cpu%d/snapshot_raw", buffer, cpu);
+		ret = asprintf(&path, "%s/per_cpu/cpu%d/snapshot_raw", buffer, cpu);
 	else
-		sprintf(path, "%s/per_cpu/cpu%d/trace_pipe_raw", buffer, cpu);
+		ret = asprintf(&path, "%s/per_cpu/cpu%d/trace_pipe_raw", buffer, cpu);
+	if (ret < 0)
+		goto out_free;
+
 	recorder->trace_fd = open(path, O_RDONLY);
 	if (recorder->trace_fd < 0)
 		goto out_free;
diff --git a/trace-show.c b/trace-show.c
index 14b786c..f13db31 100644
--- a/trace-show.c
+++ b/trace-show.c
@@ -154,11 +154,11 @@ void trace_show(int argc, char **argv)
 	}
 
 	if (buffer) {
-		path = malloc(strlen(buffer) + strlen("instances//") +
-			      strlen(file) + 1);
-		if (!path)
+		int ret;
+
+		ret = asprintf(&path, "instances/%s/%s", buffer, file);
+		if (ret < 0)
 			die("Failed to allocate instance path %s", file);
-		sprintf(path, "instances/%s/%s", buffer, file);
 		file = path;
 	}
 
diff --git a/trace-split.c b/trace-split.c
index 87d43ad..5e4ac68 100644
--- a/trace-split.c
+++ b/trace-split.c
@@ -369,10 +369,11 @@ static double parse_file(struct tracecmd_input *handle,
 		die("Failed to allocate cpu_data for %d cpus", cpus);
 
 	for (cpu = 0; cpu < cpus; cpu++) {
-		file = malloc(strlen(output_file) + 50);
-		if (!file)
+		int ret;
+
+		ret = asprintf(&file, "%s/.tmp.%s.%d", dir, base, cpu);
+		if (ret < 0)
 			die("Failed to allocate file for %s %s %d", dir, base, cpu);
-		sprintf(file, "%s/.tmp.%s.%d", dir, base, cpu);
 		fd = open(file, O_WRONLY | O_CREAT | O_TRUNC | O_LARGEFILE, 0644);
 		cpu_data[cpu].cpu = cpu;
 		cpu_data[cpu].fd = fd;
diff --git a/trace-stat.c b/trace-stat.c
index adbf3c3..fd16354 100644
--- a/trace-stat.c
+++ b/trace-stat.c
@@ -70,15 +70,16 @@ char *strstrip(char *str)
 	return s;
 }
 
+/* FIXME: append_file() is duplicated and could be consolidated */
 char *append_file(const char *dir, const char *name)
 {
 	char *file;
+	int ret;
 
-	file = malloc(strlen(dir) + strlen(name) + 2);
-	if (!file)
+	ret = asprintf(&file, "%s/%s", dir, name);
+	if (ret < 0)
 		die("Failed to allocate %s/%s", dir, name);
 
-	sprintf(file, "%s/%s", dir, name);
 	return file;
 }
 
diff --git a/trace-util.c b/trace-util.c
index d464a61..45fa95a 100644
--- a/trace-util.c
+++ b/trace-util.c
@@ -85,14 +85,15 @@ char **trace_util_list_plugin_options(void)
 	for (reg = registered_options; reg; reg = reg->next) {
 		for (op = reg->options; op->name; op++) {
 			char *alias = op->plugin_alias ? op->plugin_alias : op->file;
+			int ret;
 
-			name = malloc(strlen(op->name) + strlen(alias) + 2);
-			if (!name) {
+			ret = asprintf(&name, "%s:%s", alias, op->name);
+			if (ret < 0) {
 				warning("Failed to allocate plugin option %s:%s",
 					alias, op->name);
 				break;
 			}
-			sprintf(name, "%s:%s", alias, op->name);
+
 			list = realloc(list, count + 2);
 			if (!list) {
 				warning("Failed to allocate plugin list for %s", name);
@@ -614,14 +615,10 @@ static int load_plugin(struct pevent *pevent, const char *path,
 	void *handle;
 	int ret;
 
-	plugin = malloc(strlen(path) + strlen(file) + 2);
-	if (!plugin)
+	ret = asprintf(&plugin, "%s/%s", path, file);
+	if (ret < 0)
 		return -ENOMEM;
 
-	strcpy(plugin, path);
-	strcat(plugin, "/");
-	strcat(plugin, file);
-
 	handle = dlopen(plugin, RTLD_NOW | RTLD_GLOBAL);
 	if (!handle) {
 		warning("cound not load plugin '%s'\n%s\n",
@@ -707,7 +704,7 @@ char *tracecmd_find_tracing_dir(void)
 	char type[100];
 	int use_debug = 0;
 	FILE *fp;
-	
+
 	if ((fp = fopen("/proc/mounts","r")) == NULL) {
 		warning("Can't open /proc/mounts for read");
 		return NULL;
@@ -749,16 +746,16 @@ char *tracecmd_find_tracing_dir(void)
 	free(debug_str);
 
 	if (use_debug) {
-		tracing_dir = malloc(strlen(fspath) + 9);
-		if (!tracing_dir)
-			return NULL;
+		int ret;
 
-		sprintf(tracing_dir, "%s/tracing", fspath);
+		ret = asprintf(&tracing_dir, "%s/tracing", fspath);
+		if (ret < 0)
+			return NULL;
 	} else {
 		tracing_dir = strdup(fspath);
 		if (!tracing_dir)
 			return NULL;
-	}		
+	}
 
 	return tracing_dir;
 }
@@ -774,16 +771,15 @@ const char *tracecmd_get_tracing_dir(void)
 	return tracing_dir;
 }
 
+/* FIXME: append_file() is duplicated and could be consolidated */
 static char *append_file(const char *dir, const char *name)
 {
 	char *file;
+	int ret;
 
-	file = malloc(strlen(dir) + strlen(name) + 2);
-	if (!file)
-		return NULL;
+	ret = asprintf(&file, "%s/%s", dir, name);
 
-	sprintf(file, "%s/%s", dir, name);
-	return file;
+	return ret < 0 ? NULL : file;
 }
 
 /**
@@ -1389,7 +1385,8 @@ int trace_util_load_plugins(struct pevent *pevent, const char *suffix,
 {
 	char *home;
 	char *path;
-        char *envdir;
+	char *envdir;
+	int ret;
 
 	if (tracecmd_disable_plugins)
 		return -EBUSY;
@@ -1412,14 +1409,10 @@ int trace_util_load_plugins(struct pevent *pevent, const char *suffix,
 	if (!home)
 		return -EINVAL;
 
-	path = malloc(strlen(home) + strlen(LOCAL_PLUGIN_DIR) + 2);
-	if (!path)
+	ret = asprintf(&path, "%s/%s", home, LOCAL_PLUGIN_DIR);
+	if (ret < 0)
 		return -ENOMEM;
 
-	strcpy(path, home);
-	strcat(path, "/");
-	strcat(path, LOCAL_PLUGIN_DIR);
-
 	trace_util_load_plugins_dir(pevent, suffix, path, load_plugin, data);
 
 	free(path);
@@ -1506,15 +1499,12 @@ static int read_options(struct pevent *pevent, const char *path,
 	int unload = 0;
 	char *plugin;
 	void *handle;
+	int ret;
 
-	plugin = malloc(strlen(path) + strlen(file) + 2);
-	if (!plugin)
+	ret = asprintf(&plugin, "%s/%s", path, file);
+	if (ret < 0)
 		return -ENOMEM;
 
-	strcpy(plugin, path);
-	strcat(plugin, "/");
-	strcat(plugin, file);
-
 	handle = dlopen(plugin, RTLD_NOW | RTLD_GLOBAL);
 	if (!handle) {
 		warning("cound not load plugin '%s'\n%s\n",
@@ -1603,6 +1593,7 @@ char *tracecmd_get_tracing_file(const char *name)
 {
 	static const char *tracing;
 	char *file;
+	int ret;
 
 	if (!tracing) {
 		tracing = tracecmd_find_tracing_dir();
@@ -1610,11 +1601,10 @@ char *tracecmd_get_tracing_file(const char *name)
 			return NULL;
 	}
 
-	file = malloc(strlen(tracing) + strlen(name) + 2);
-	if (!file)
+	ret = asprintf(&file, "%s/%s", tracing, name);
+	if (ret < 0)
 		return NULL;
 
-	sprintf(file, "%s/%s", tracing, name);
 	return file;
 }
 
-- 
2.13.3

Re: [PATCH V3 2/2] It makes the code clearer and less error prone.

[Steven Rostedt]

On Thu,  3 Aug 2017 00:15:58 +0200
Federico Vaga <federico.vaga@vaga.pv.it> wrote:

Why did you change the subject? The previous patch had a much better
one: "trace-cmd: Use asprintf when possible"

Or was it the tool you used to send the patches that chopped it off?

A quick scan of the patch looks good. I'll look a bit deeper at it, and
if I don't find anything, I'll apply it (and fix the subject).

Thanks!

-- Steve

> clearer:
> - less code
> - the code is now using the same format to create strings dynamically
> 
> less error prone:
> - no magic number +2 +9 +5 to compute the size
> - no copy&paste of the strings to compute the size and to concatenate
> 
> The function `asprintf` is not POSIX standard but the program
> was already using it. Later it can be decided to use only POSIX
> functions, then we can easly replace all the `asprintf(3)` with a local
> implementation of that function.
> 
> Signed-off-by: Federico Vaga <federico.vaga@vaga.pv.it>
> ---
>  event-plugin.c   | 24 ++++++++--------------
>

Re: [PATCH V3 2/2] It makes the code clearer and less error prone.

[Federico Vaga]

On Monday, August 14, 2017 7:33:41 PM CEST Steven Rostedt wrote:
> On Thu,  3 Aug 2017 00:15:58 +0200
> Federico Vaga <federico.vaga@vaga.pv.it> wrote:
> 
> Why did you change the subject? The previous patch had a much better
> one: "trace-cmd: Use asprintf when possible"
>
> Or was it the tool you used to send the patches that chopped it off?

It was not my intention.
I used git send-email, but probably I did something wrong by accident. Sorry

> 
> A quick scan of the patch looks good. I'll look a bit deeper at it, and
> if I don't find anything, I'll apply it (and fix the subject).

Ok, thank you

-- 
Federico Vaga
http://www.federicovaga.it/

Re: [PATCH V3 2/2] It makes the code clearer and less error prone.

[Steven Rostedt]

On Tue, 15 Aug 2017 09:25:10 +0200
Federico Vaga <federico.vaga@vaga.pv.it> wrote:

> On Monday, August 14, 2017 7:33:41 PM CEST Steven Rostedt wrote:
> > On Thu,  3 Aug 2017 00:15:58 +0200
> > Federico Vaga <federico.vaga@vaga.pv.it> wrote:
> > 
> > Why did you change the subject? The previous patch had a much better
> > one: "trace-cmd: Use asprintf when possible"
> >
> > Or was it the tool you used to send the patches that chopped it off?  
> 
> It was not my intention.
> I used git send-email, but probably I did something wrong by accident. Sorry
> 
> > 
> > A quick scan of the patch looks good. I'll look a bit deeper at it, and
> > if I don't find anything, I'll apply it (and fix the subject).  
> 
> Ok, thank you
> 

Finally got around to applying your patches. Sorry for the long
delay. :-/  I've been traveling too much lately.

-- Steve

Re: [PATCH V3 1/2] trace-cmd record: use direname instead of custom code

[Steven Rostedt]

On Thu,  3 Aug 2017 00:15:57 +0200
Federico Vaga <federico.vaga@vaga.pv.it> wrote:

> Prefer well known functions like `dirname(3)` instead of custom
> implementation for the same functionality
> 
> Signed-off-by: Federico Vaga <federico.vaga@vaga.pv.it>

Thanks. I'll be traveling for a bit and wont get to these before I
leave. Just a note. When sending patches, can you add to the subject,
something like I just did "trace-cmd record:" to let those know that
this is a trace-cmd patch (especially when sent to LKML). It also makes
it easier for me when I look for trace-cmd patches, as I will filter on
"trace-cmd" in the subject.

Thanks,

-- Steve

Re: [PATCH V3 1/2] use direname instead of custom code

[Steven Rostedt]

I'm back from vacation!

On Thu,  3 Aug 2017 00:15:57 +0200
Federico Vaga <federico.vaga@vaga.pv.it> wrote:

> Prefer well known functions like `dirname(3)` instead of custom
> implementation for the same functionality
> 
> Signed-off-by: Federico Vaga <federico.vaga@vaga.pv.it>
> ---
>  trace-record.c | 45 ++++++++++++++++++++++++++++++---------------
>  1 file changed, 30 insertions(+), 15 deletions(-)
> 
> diff --git a/trace-record.c b/trace-record.c
> index 020a373..79e4fe4 100644
> --- a/trace-record.c
> +++ b/trace-record.c
> @@ -45,6 +45,7 @@
>  #include <glob.h>
>  #include <errno.h>
>  #include <limits.h>
> +#include <libgen.h>
>  
>  #include "trace-local.h"
>  #include "trace-msg.h"
> @@ -2215,12 +2216,24 @@ static void set_max_graph_depth(struct buffer_instance *instance, char *max_grap
>  		die("could not write to max_graph_depth");
>  }
>  
> +
> +/**
> + * create_event - create and event descriptor
> + * @instance: instance to use
> + * @path: path to event attribute
> + * @old_event: event descriptor to use as base
> + *
> + * NOTE: the function purpose is to create a data structure to describe
> + * an ftrace event. During the process it becomes handy to change the
> + * string `path`. So, do not rely on the content of `path` after you
> + * invoke this function.
> + */
>  static struct event_list *
>  create_event(struct buffer_instance *instance, char *path, struct event_list *old_event)
>  {
>  	struct event_list *event;
>  	struct stat st;
> -	char *p;
> +	char *p, *path_dirname;

Just a personal preference, but I prefer to have each variable on its
own line, with a few exceptions. Makes diffs better when updating them.

>  	int ret;
>  
>  	event = malloc(sizeof(*event));
> @@ -2234,14 +2247,14 @@ create_event(struct buffer_instance *instance, char *path, struct event_list *ol
>  		if (!event->filter_file)
>  			die("malloc filter file");
>  	}
> -	for (p = path + strlen(path) - 1; p > path; p--)
> -		if (*p == '/')
> -			break;
> -	*p = '\0';
> -	p = malloc(strlen(path) + strlen("/enable") + 1);
> +
> +	path_dirname = dirname(path);
> +
> +	p = malloc(strlen(path_dirname) + strlen("/enable") + 1);
>  	if (!p)
>  		die("Failed to allocate enable path for %s", path);
> -	sprintf(p, "%s/enable", path);
> +	sprintf(p, "%s/enable", path_dirname);
> +
>  	ret = stat(p, &st);
>  	if (ret >= 0)
>  		event->enable_file = p;
> @@ -2249,10 +2262,11 @@ create_event(struct buffer_instance *instance, char *path, struct event_list *ol
>  		free(p);
>  
>  	if (event->trigger) {
> -		p = malloc(strlen(path) + strlen("/trigger") + 1);
> +		p = malloc(strlen(path_dirname) + strlen("/trigger") + 1);
>  		if (!p)
>  			die("Failed to allocate trigger path for %s", path);
> -		sprintf(p, "%s/trigger", path);
> +		sprintf(p, "%s/trigger", path_dirname);
> +
>  		ret = stat(p, &st);
>  		if (ret > 0)
>  			die("trigger specified but not supported by this kernel");
> @@ -2266,8 +2280,7 @@ static void make_sched_event(struct buffer_instance *instance,
>  			     struct event_list **event, struct event_list *sched,
>  			     const char *sched_path)
>  {
> -	char *path;
> -	char *p;
> +	char *path, *path_dirname, *sched_filter_file_tmp;

These should be broken up too. Also, "sched_filter_file_tmp" is a bit
too descriptive.  Something like "tmp_file" should suffice.

Thanks!

-- Steve

>  
>  	/* Do nothing if the event already exists */
>  	if (*event)
> @@ -2277,11 +2290,13 @@ static void make_sched_event(struct buffer_instance *instance,
>  	if (!path)
>  		die("Failed to allocate path for %s", sched_path);
>  
> -	sprintf(path, "%s", sched->filter_file);
> +	/* we do not want to corrupt sched->filter_file when using dirname() */
> +	sched_filter_file_tmp = strdup(sched->filter_file);
> +	if (!sched_filter_file_tmp)
> +		die("Failed to allocate path for %s", sched_path);
> +	path_dirname = dirname(sched_filter_file_tmp);
>  
> -	/* Remove the /filter from filter file */
> -	p = path + strlen(path) - strlen("filter");
> -	sprintf(p, "%s/filter", sched_path);
> +	sprintf(path, "%s/%s/filter", path_dirname, sched_path);
>  
>  	*event = create_event(instance, path, sched);
>  	free(path);

[PATCH v3 0/6] trace-cmd: escape sequence, EINTR, error checking bug fixes

[Michael Sartain]

This patch adds fixes to trace-cmd for return value checking, EINTR handling,
function prototypes, and data offsets to initial patch escape sequence fix [1].

Thanks much.
 -Mike

v2->v3:
  Fix attachments (aka learn how to use Mutt)
v1->v2:
  Add five related bug fix patches

Michael Sartain (6):
  Fix bad force_token escape sequence
  Fix unsigned return values being error checked as negative
  Handle EINTR signal interrupts for read, write, open calls
  Fix read / write data offsets in read / write loops
  Fix function prototypes for __vwarning, __vpr_stat, and __vdie
  Fix cases where string literals were passed as string format args

 event-parse.c     |   2 +-
 event-utils.h     |   4 +-
 parse-utils.c     |   2 +
 trace-capture.c   |  12 ++--
 trace-cmd-local.h |   2 +-
 trace-dialog.c    |   4 +-
 trace-filter.c    |  10 +--
 trace-input.c     | 187 +++++++++++++++++++++++-------------------------------
 trace-local.h     |   2 +-
 trace-msg.c       |  13 ++--
 10 files changed, 104 insertions(+), 134 deletions(-)

[1] https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1414382.html

-- 
2.11.0

[PATCH v3 1/6] Fix bad force_token escape sequence

[Michael Sartain]

Signed-off-by: Michael Sartain <mikesart@fastmail.com>
---
 event-parse.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/event-parse.c b/event-parse.c
index 3217131..61f48c1 100644
--- a/event-parse.c
+++ b/event-parse.c
@@ -1093,7 +1093,7 @@ static enum event_type __read_token(char **tok)
 		if (strcmp(*tok, "LOCAL_PR_FMT") == 0) {
 			free(*tok);
 			*tok = NULL;
-			return force_token("\"\%s\" ", tok);
+			return force_token("\"%s\" ", tok);
 		} else if (strcmp(*tok, "STA_PR_FMT") == 0) {
 			free(*tok);
 			*tok = NULL;
-- 
2.11.0

[tip:perf/core] tools lib traceevent: Fix bad force_token escape sequence

[tip-bot for Michael Sartain]

Commit-ID:  952a99ccfa9db2f9a32810fc9c0084f532dd871a
Gitweb:     https://git.kernel.org/tip/952a99ccfa9db2f9a32810fc9c0084f532dd871a
Author:     Michael Sartain <mikesart@fastmail.com>
AuthorDate: Thu, 11 Jan 2018 19:47:42 -0500
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 17 Jan 2018 10:21:39 -0300

tools lib traceevent: Fix bad force_token escape sequence

Older kernels have a bug that creates invalid symbols. event-parse.c
handles them by replacing them with a "%s" token. But the fix included
an extra backslash, and "\%s" was added incorrectly.

Signed-off-by: Michael Sartain <mikesart@fastmail.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Link: http://lkml.kernel.org/r/20180112004821.827168881@goodmis.org
Link: http://lkml.kernel.org/r/d320000d37c10ce0912851e1fb78d1e0c946bcd9.1497486273.git.mikesart@fastmail.com
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/traceevent/event-parse.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 7ce724f..0bc1a6d 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -1094,7 +1094,7 @@ static enum event_type __read_token(char **tok)
 		if (strcmp(*tok, "LOCAL_PR_FMT") == 0) {
 			free(*tok);
 			*tok = NULL;
-			return force_token("\"\%s\" ", tok);
+			return force_token("\"%s\" ", tok);
 		} else if (strcmp(*tok, "STA_PR_FMT") == 0) {
 			free(*tok);
 			*tok = NULL;

[PATCH v3 2/6] Fix unsigned return values being error checked as negative

[Michael Sartain]

Functions like read4 and read8 had unsigned return types but callers were
checking for those return values being less than 0 for errors.

This patch changes those functions to return signed int error values and take a
pointer to a size parameter. Also changes several locals to match the function
types.

Signed-off-by: Michael Sartain <mikesart@fastmail.com>
---
 trace-input.c | 179 ++++++++++++++++++++++++----------------------------------
 trace-msg.c   |   5 +-
 2 files changed, 78 insertions(+), 106 deletions(-)

diff --git a/trace-input.c b/trace-input.c
index e676c85..89ddcf5 100644
--- a/trace-input.c
+++ b/trace-input.c
@@ -196,10 +196,10 @@ static const char *show_records(struct list_head *pages)
 
 static int init_cpu(struct tracecmd_input *handle, int cpu);
 
-static int do_read(struct tracecmd_input *handle, void *data, int size)
+static ssize_t do_read(struct tracecmd_input *handle, void *data, size_t size)
 {
-	int tot = 0;
-	int r;
+	ssize_t tot = 0;
+	ssize_t r;
 
 	do {
 		r = read(handle->fd, data, size - tot);
@@ -214,10 +214,10 @@ static int do_read(struct tracecmd_input *handle, void *data, int size)
 	return tot;
 }
 
-static int
-do_read_check(struct tracecmd_input *handle, void *data, int size)
+static ssize_t
+do_read_check(struct tracecmd_input *handle, void *data, size_t size)
 {
-	int ret;
+	ssize_t ret;
 
 	ret = do_read(handle, data, size);
 	if (ret < 0)
@@ -232,9 +232,9 @@ static char *read_string(struct tracecmd_input *handle)
 {
 	char buf[BUFSIZ];
 	char *str = NULL;
-	int size = 0;
-	int i;
-	int r;
+	size_t size = 0;
+	ssize_t i;
+	ssize_t r;
 
 	for (;;) {
 		r = do_read(handle, buf, BUFSIZ);
@@ -294,7 +294,7 @@ static char *read_string(struct tracecmd_input *handle)
 	return NULL;
 }
 
-static unsigned int read4(struct tracecmd_input *handle)
+static int read4(struct tracecmd_input *handle, unsigned int *size)
 {
 	struct pevent *pevent = handle->pevent;
 	unsigned int data;
@@ -302,10 +302,11 @@ static unsigned int read4(struct tracecmd_input *handle)
 	if (do_read_check(handle, &data, 4))
 		return -1;
 
-	return __data2host4(pevent, data);
+	*size = __data2host4(pevent, data);
+	return 0;
 }
 
-static unsigned long long read8(struct tracecmd_input *handle)
+static int read8(struct tracecmd_input *handle, unsigned long long *size)
 {
 	struct pevent *pevent = handle->pevent;
 	unsigned long long data;
@@ -313,13 +314,14 @@ static unsigned long long read8(struct tracecmd_input *handle)
 	if (do_read_check(handle, &data, 8))
 		return -1;
 
-	return __data2host8(pevent, data);
+	*size = __data2host8(pevent, data);
+	return 0;
 }
 
 static int read_header_files(struct tracecmd_input *handle)
 {
 	struct pevent *pevent = handle->pevent;
-	long long size;
+	unsigned long long size;
 	char *header;
 	char buf[BUFSIZ];
 
@@ -329,8 +331,7 @@ static int read_header_files(struct tracecmd_input *handle)
 	if (memcmp(buf, "header_page", 12) != 0)
 		return -1;
 
-	size = read8(handle);
-	if (size < 0)
+	if (read8(handle, &size) < 0)
 		return -1;
 
 	header = malloc(size);
@@ -355,8 +356,7 @@ static int read_header_files(struct tracecmd_input *handle)
 	if (memcmp(buf, "header_event", 13) != 0)
 		return -1;
 
-	size = read8(handle);
-	if (size < 0)
+	if (read8(handle, &size) < 0)
 		return -1;
 
 	header = malloc(size);
@@ -521,11 +521,10 @@ static int read_ftrace_files(struct tracecmd_input *handle, const char *regex)
 	regex_t epreg;
 	regex_t *sreg = NULL;
 	regex_t *ereg = NULL;
+	unsigned int count, i;
 	int print_all = 0;
 	int unique;
-	int count;
 	int ret;
-	int i;
 
 	if (regex) {
 		sreg = &spreg;
@@ -554,13 +553,11 @@ static int read_ftrace_files(struct tracecmd_input *handle, const char *regex)
 		}
 	}
 
-	count = read4(handle);
-	if (count < 0)
+	if (read4(handle, &count) < 0)
 		return -1;
 
 	for (i = 0; i < count; i++) {
-		size = read8(handle);
-		if (size < 0)
+		if (read8(handle, &size) < 0)
 			return -1;
 		ret = read_ftrace_file(handle, size, print_all, ereg);
 		if (ret < 0)
@@ -587,13 +584,13 @@ static int read_event_files(struct tracecmd_input *handle, const char *regex)
 	regex_t *sreg = NULL;
 	regex_t *ereg = NULL;
 	regex_t *reg;
-	int systems;
+	unsigned int systems;
+	unsigned int count;
+	unsigned int i, x;
 	int print_all;
 	int sys_printed;
-	int count;
 	int unique;
 	int ret;
-	int i,x;
 
 	if (regex) {
 		sreg = &spreg;
@@ -603,8 +600,7 @@ static int read_event_files(struct tracecmd_input *handle, const char *regex)
 			return -1;
 	}
 
-	systems = read4(handle);
-	if (systems < 0)
+	if (read4(handle, &systems) < 0)
 		return -1;
 
 	for (i = 0; i < systems; i++) {
@@ -637,13 +633,11 @@ static int read_event_files(struct tracecmd_input *handle, const char *regex)
 			}
 		}
 
-		count = read4(handle);
-		if (count < 0)
+		if (read4(handle, &count) < 0)
 			goto failed;
 
 		for (x=0; x < count; x++) {
-			size = read8(handle);
-			if (size < 0)
+			if (read8(handle, &size) < 0)
 				goto failed;
 
 			ret = read_event_file(handle, system, size,
@@ -675,16 +669,14 @@ static int read_event_files(struct tracecmd_input *handle, const char *regex)
 static int read_proc_kallsyms(struct tracecmd_input *handle)
 {
 	struct pevent *pevent = handle->pevent;
-	int size;
+	unsigned int size;
 	char *buf;
 
-	size = read4(handle);
+	if (read4(handle, &size) < 0)
+		return -1;
 	if (!size)
 		return 0; /* OK? */
 
-	if (size < 0)
-		return -1;
-
 	buf = malloc(size+1);
 	if (!buf)
 		return -1;
@@ -702,16 +694,14 @@ static int read_proc_kallsyms(struct tracecmd_input *handle)
 
 static int read_ftrace_printk(struct tracecmd_input *handle)
 {
-	int size;
+	unsigned int size;
 	char *buf;
 
-	size = read4(handle);
+	if (read4(handle, &size) < 0)
+		return -1;
 	if (!size)
 		return 0; /* OK? */
 
-	if (size < 0)
-		return -1;
-
 	buf = malloc(size + 1);
 	if (!buf)
 		return -1;
@@ -2268,8 +2258,8 @@ static int read_cpu_data(struct tracecmd_input *handle)
 		if (pevent->old_format)
 			kbuffer_set_old_format(handle->cpu_data[cpu].kbuf);
 
-		offset = read8(handle);
-		size = read8(handle);
+		read8(handle, &offset);
+		read8(handle, &size);
 
 		handle->cpu_data[cpu].file_offset = offset;
 		handle->cpu_data[cpu].file_size = size;
@@ -2315,8 +2305,7 @@ static int read_cpu_data(struct tracecmd_input *handle)
 static int read_data_and_size(struct tracecmd_input *handle,
 				     char **data, unsigned long long *size)
 {
-	*size = read8(handle);
-	if (*size < 0)
+	if (read8(handle, size) < 0)
 		return -1;
 	*data = malloc(*size + 1);
 	if (!*data)
@@ -2367,11 +2356,12 @@ static int read_and_parse_trace_clock(struct tracecmd_input *handle,
 int tracecmd_init_data(struct tracecmd_input *handle)
 {
 	struct pevent *pevent = handle->pevent;
+    unsigned int cpus;
 	int ret;
 
-	handle->cpus = read4(handle);
-	if (handle->cpus < 0)
+	if (read4(handle, &cpus) < 0)
 		return -1;
+	handle->cpus = cpus;
 
 	pevent_set_cpus(pevent, handle->cpus);
 
@@ -2570,6 +2560,7 @@ struct tracecmd_input *tracecmd_alloc_fd(int fd)
 {
 	struct tracecmd_input *handle;
 	char test[] = { 23, 8, 68 };
+    unsigned int page_size;
 	char *version;
 	char buf[BUFSIZ];
 
@@ -2616,7 +2607,8 @@ struct tracecmd_input *tracecmd_alloc_fd(int fd)
 	do_read_check(handle, buf, 1);
 	handle->long_size = buf[0];
 
-	handle->page_size = read4(handle);
+	read4(handle, &page_size);
+	handle->page_size = page_size;
 
 	handle->header_files_start =
 		lseek64(handle->fd, 0, SEEK_CUR);
@@ -2772,36 +2764,30 @@ void tracecmd_close(struct tracecmd_input *handle)
 	free(handle);
 }
 
-static long long read_copy_size8(struct tracecmd_input *handle, int fd)
+static int read_copy_size8(struct tracecmd_input *handle, int fd, unsigned long long *size)
 {
-	long long size;
-
 	/* read size */
-	if (do_read_check(handle, &size, 8))
+	if (do_read_check(handle, size, 8))
 		return -1;
 
-	if (__do_write_check(fd, &size, 8))
+	if (__do_write_check(fd, size, 8))
 		return -1;
 
-	size = __data2host8(handle->pevent, size);
-
-	return size;
+	*size = __data2host8(handle->pevent, *size);
+	return 0;
 }
 
-static int read_copy_size4(struct tracecmd_input *handle, int fd)
+static int read_copy_size4(struct tracecmd_input *handle, int fd, unsigned int *size)
 {
-	int size;
-
 	/* read size */
-	if (do_read_check(handle, &size, 4))
+	if (do_read_check(handle, size, 4))
 		return -1;
 
-	if (__do_write_check(fd, &size, 4))
+	if (__do_write_check(fd, size, 4))
 		return -1;
 
-	size = __data2host4(handle->pevent, size);
-
-	return size;
+	*size = __data2host4(handle->pevent, *size);
+	return 0;
 }
 
 static int read_copy_data(struct tracecmd_input *handle,
@@ -2829,7 +2815,7 @@ static int read_copy_data(struct tracecmd_input *handle,
 
 static int copy_header_files(struct tracecmd_input *handle, int fd)
 {
-	long long size;
+	unsigned long long size;
 
 	lseek64(handle->fd, handle->header_files_start, SEEK_SET);
 
@@ -2837,8 +2823,7 @@ static int copy_header_files(struct tracecmd_input *handle, int fd)
 	if (read_copy_data(handle, 12, fd) < 0)
 		return -1;
 
-	size = read_copy_size8(handle, fd);
-	if (size < 0)
+	if (read_copy_size8(handle, fd, &size) < 0)
 		return -1;
 
 	if (read_copy_data(handle, size, fd) < 0)
@@ -2848,8 +2833,7 @@ static int copy_header_files(struct tracecmd_input *handle, int fd)
 	if (read_copy_data(handle, 13, fd) < 0)
 		return -1;
 
-	size = read_copy_size8(handle, fd);
-	if (size < 0)
+	if (read_copy_size8(handle, fd, &size) < 0)
 		return -1;
 
 	if (read_copy_data(handle, size, fd) < 0)
@@ -2861,17 +2845,15 @@ static int copy_header_files(struct tracecmd_input *handle, int fd)
 static int copy_ftrace_files(struct tracecmd_input *handle, int fd)
 {
 	unsigned long long size;
-	int count;
-	int i;
+	unsigned int count;
+	unsigned int i;
 
-	count = read_copy_size4(handle, fd);
-	if (count < 0)
+	if (read_copy_size4(handle, fd, &count) < 0)
 		return -1;
 
 	for (i = 0; i < count; i++) {
 
-		size = read_copy_size8(handle, fd);
-		if (size < 0)
+		if (read_copy_size8(handle, fd, &size) < 0)
 			return -1;
 
 		if (read_copy_data(handle, size, fd) < 0)
@@ -2885,13 +2867,11 @@ static int copy_event_files(struct tracecmd_input *handle, int fd)
 {
 	unsigned long long size;
 	char *system;
-	int systems;
-	int count;
-	int ret;
-	int i,x;
+	unsigned int systems;
+	unsigned int count;
+	unsigned int i,x;
 
-	systems = read_copy_size4(handle, fd);
-	if (systems < 0)
+	if (read_copy_size4(handle, fd, &systems) < 0)
 		return -1;
 
 	for (i = 0; i < systems; i++) {
@@ -2904,17 +2884,14 @@ static int copy_event_files(struct tracecmd_input *handle, int fd)
 		}
 		free(system);
 
-		count = read_copy_size4(handle, fd);
-		if (count < 0)
+		if (read_copy_size4(handle, fd, &count) < 0)
 			return -1;
 
 		for (x=0; x < count; x++) {
-			size = read_copy_size8(handle, fd);
-			if (size < 0)
+			if (read_copy_size8(handle, fd, &size) < 0)
 				return -1;
 
-			ret = read_copy_data(handle, size, fd);
-			if (ret < 0)
+			if (read_copy_data(handle, size, fd) < 0)
 				return -1;
 		}
 	}
@@ -2924,15 +2901,13 @@ static int copy_event_files(struct tracecmd_input *handle, int fd)
 
 static int copy_proc_kallsyms(struct tracecmd_input *handle, int fd)
 {
-	int size;
+	unsigned int size;
 
-	size = read_copy_size4(handle, fd);
+	if (read_copy_size4(handle, fd, &size) < 0)
+		return -1;
 	if (!size)
 		return 0; /* OK? */
 
-	if (size < 0)
-		return -1;
-
 	if (read_copy_data(handle, size, fd) < 0)
 		return -1;
 
@@ -2941,15 +2916,13 @@ static int copy_proc_kallsyms(struct tracecmd_input *handle, int fd)
 
 static int copy_ftrace_printk(struct tracecmd_input *handle, int fd)
 {
-	int size;
+	unsigned int size;
 
-	size = read_copy_size4(handle, fd);
+	if (read_copy_size4(handle, fd, &size) < 0)
+		return -1;
 	if (!size)
 		return 0; /* OK? */
 
-	if (size < 0)
-		return -1;
-
 	if (read_copy_data(handle, size, fd) < 0)
 		return -1;
 
@@ -2958,15 +2931,13 @@ static int copy_ftrace_printk(struct tracecmd_input *handle, int fd)
 
 static int copy_command_lines(struct tracecmd_input *handle, int fd)
 {
-	unsigned long size;
+	unsigned long long size;
 
-	size = read_copy_size8(handle, fd);
+	if (read_copy_size8(handle, fd, &size) < 0)
+		return -1;
 	if (!size)
 		return 0; /* OK? */
 
-	if (size < 0)
-		return -1;
-
 	if (read_copy_data(handle, size, fd) < 0)
 		return -1;
 
diff --git a/trace-msg.c b/trace-msg.c
index f1b6546..3991985 100644
--- a/trace-msg.c
+++ b/trace-msg.c
@@ -288,7 +288,7 @@ static int tracecmd_msg_send(int fd, u32 cmd)
 
 static int msg_read(int fd, void *buf, u32 size, int *n)
 {
-	int r;
+	ssize_t r;
 
 	while (size) {
 		r = read(fd, buf + *n, size);
@@ -637,7 +637,8 @@ int tracecmd_msg_finish_sending_metadata(int fd)
 int tracecmd_msg_collect_metadata(int ifd, int ofd)
 {
 	struct tracecmd_msg msg;
-	u32 s, t, n, cmd;
+	u32 t, n, cmd;
+	ssize_t s;
 	int ret;
 
 	do {
-- 
2.11.0

[PATCH v3 3/6] Handle EINTR signal interrupts for read, write, open calls

[Michael Sartain]

Read/write/open calls weren't handling EINTR in trace-input.c

This patch uses the standard GNU C TEMP_FAILURE_RETRY macro to handle EINTR
return values, and updates read/write calls in trace-msg.c to match.

Signed-off-by: Michael Sartain <mikesart@fastmail.com>
---
 trace-cmd-local.h | 2 +-
 trace-input.c     | 8 ++++----
 trace-msg.c       | 8 ++------
 3 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/trace-cmd-local.h b/trace-cmd-local.h
index 9412f9d..8595a8a 100644
--- a/trace-cmd-local.h
+++ b/trace-cmd-local.h
@@ -31,7 +31,7 @@ static ssize_t __do_write(int fd, const void *data, size_t size)
 	ssize_t w;
 
 	do {
-		w = write(fd, data, size - tot);
+		w = TEMP_FAILURE_RETRY(write(fd, data, size - tot));
 		tot += w;
 
 		if (!w)
diff --git a/trace-input.c b/trace-input.c
index 89ddcf5..251d32b 100644
--- a/trace-input.c
+++ b/trace-input.c
@@ -202,7 +202,7 @@ static ssize_t do_read(struct tracecmd_input *handle, void *data, size_t size)
 	ssize_t r;
 
 	do {
-		r = read(handle->fd, data, size - tot);
+		r = TEMP_FAILURE_RETRY(read(handle->fd, data, size - tot));
 		tot += r;
 
 		if (!r)
@@ -774,7 +774,7 @@ static int read_page(struct tracecmd_input *handle, off64_t offset,
 	off64_t ret;
 
 	if (handle->use_pipe) {
-		ret = read(handle->cpu_data[cpu].pipe_fd, map, handle->page_size);
+		ret = TEMP_FAILURE_RETRY(read(handle->cpu_data[cpu].pipe_fd, map, handle->page_size));
 		/* Set EAGAIN if the pipe is empty */
 		if (ret < 0) {
 			errno = EAGAIN;
@@ -2645,7 +2645,7 @@ struct tracecmd_input *tracecmd_alloc(const char *file)
 {
 	int fd;
 
-	fd = open(file, O_RDONLY);
+	fd = TEMP_FAILURE_RETRY(open(file, O_RDONLY));
 	if (fd < 0)
 		return NULL;
 
@@ -2686,7 +2686,7 @@ struct tracecmd_input *tracecmd_open(const char *file)
 {
 	int fd;
 
-	fd = open(file, O_RDONLY);
+	fd = TEMP_FAILURE_RETRY(open(file, O_RDONLY));
 	if (fd < 0)
 		return NULL;
 
diff --git a/trace-msg.c b/trace-msg.c
index 3991985..d358318 100644
--- a/trace-msg.c
+++ b/trace-msg.c
@@ -291,10 +291,8 @@ static int msg_read(int fd, void *buf, u32 size, int *n)
 	ssize_t r;
 
 	while (size) {
-		r = read(fd, buf + *n, size);
+		r = TEMP_FAILURE_RETRY(read(fd, buf + *n, size));
 		if (r < 0) {
-			if (errno == EINTR)
-				continue;
 			return -errno;
 		} else if (!r)
 			return -ENOTCONN;
@@ -662,10 +660,8 @@ int tracecmd_msg_collect_metadata(int ifd, int ofd)
 		t = n;
 		s = 0;
 		do {
-			s = write(ofd, msg.data.meta.buf+s, t);
+			s = TEMP_FAILURE_RETRY(write(ofd, msg.data.meta.buf+s, t));
 			if (s < 0) {
-				if (errno == EINTR)
-					continue;
 				warning("writing to file");
 				return -errno;
 			}
-- 
2.11.0

Re: [PATCH v3 3/6] Handle EINTR signal interrupts for read, write, open calls

[Steven Rostedt]

On Wed, 14 Jun 2017 18:27:58 -0600
Michael Sartain <mikesart@fastmail.com> wrote:

> Read/write/open calls weren't handling EINTR in trace-input.c
> 
> This patch uses the standard GNU C TEMP_FAILURE_RETRY macro to handle EINTR
> return values, and updates read/write calls in trace-msg.c to match.

I understand that GNU C gives the TEMP_FAILURE_RETRY() macro, but I
find it really ugly, and takes away from the flow of the code. Perhaps
we should just add our own static inline functions of the same name:

	write_intr(), read_intr(), open_intr()

that does the same thing and use that instead. The inline functions
could even use the TEMP_FAILURE_RETRY(). I just don't want that ugly
name scattered in the .c code.

Thanks!

-- Steve


> 
> Signed-off-by: Michael Sartain <mikesart@fastmail.com>
> ---
>  trace-cmd-local.h | 2 +-
>  trace-input.c     | 8 ++++----
>  trace-msg.c       | 8 ++------
>  3 files changed, 7 insertions(+), 11 deletions(-)
> 
> diff --git a/trace-cmd-local.h b/trace-cmd-local.h
> index 9412f9d..8595a8a 100644
> --- a/trace-cmd-local.h
> +++ b/trace-cmd-local.h
> @@ -31,7 +31,7 @@ static ssize_t __do_write(int fd, const void *data, size_t size)
>  	ssize_t w;
>  
>  	do {
> -		w = write(fd, data, size - tot);
> +		w = TEMP_FAILURE_RETRY(write(fd, data, size - tot));
>  		tot += w;
>  
>  		if (!w)
> diff --git a/trace-input.c b/trace-input.c
> index 89ddcf5..251d32b 100644
> --- a/trace-input.c
> +++ b/trace-input.c
> @@ -202,7 +202,7 @@ static ssize_t do_read(struct tracecmd_input *handle, void *data, size_t size)
>  	ssize_t r;
>  
>  	do {
> -		r = read(handle->fd, data, size - tot);
> +		r = TEMP_FAILURE_RETRY(read(handle->fd, data, size - tot));
>  		tot += r;
>  
>  		if (!r)
> @@ -774,7 +774,7 @@ static int read_page(struct tracecmd_input *handle, off64_t offset,
>  	off64_t ret;
>  
>  	if (handle->use_pipe) {
> -		ret = read(handle->cpu_data[cpu].pipe_fd, map, handle->page_size);
> +		ret = TEMP_FAILURE_RETRY(read(handle->cpu_data[cpu].pipe_fd, map, handle->page_size));
>  		/* Set EAGAIN if the pipe is empty */
>  		if (ret < 0) {
>  			errno = EAGAIN;
> @@ -2645,7 +2645,7 @@ struct tracecmd_input *tracecmd_alloc(const char *file)
>  {
>  	int fd;
>  
> -	fd = open(file, O_RDONLY);
> +	fd = TEMP_FAILURE_RETRY(open(file, O_RDONLY));
>  	if (fd < 0)
>  		return NULL;
>  
> @@ -2686,7 +2686,7 @@ struct tracecmd_input *tracecmd_open(const char *file)
>  {
>  	int fd;
>  
> -	fd = open(file, O_RDONLY);
> +	fd = TEMP_FAILURE_RETRY(open(file, O_RDONLY));
>  	if (fd < 0)
>  		return NULL;
>  
> diff --git a/trace-msg.c b/trace-msg.c
> index 3991985..d358318 100644
> --- a/trace-msg.c
> +++ b/trace-msg.c
> @@ -291,10 +291,8 @@ static int msg_read(int fd, void *buf, u32 size, int *n)
>  	ssize_t r;
>  
>  	while (size) {
> -		r = read(fd, buf + *n, size);
> +		r = TEMP_FAILURE_RETRY(read(fd, buf + *n, size));
>  		if (r < 0) {
> -			if (errno == EINTR)
> -				continue;
>  			return -errno;
>  		} else if (!r)
>  			return -ENOTCONN;
> @@ -662,10 +660,8 @@ int tracecmd_msg_collect_metadata(int ifd, int ofd)
>  		t = n;
>  		s = 0;
>  		do {
> -			s = write(ofd, msg.data.meta.buf+s, t);
> +			s = TEMP_FAILURE_RETRY(write(ofd, msg.data.meta.buf+s, t));
>  			if (s < 0) {
> -				if (errno == EINTR)
> -					continue;
>  				warning("writing to file");
>  				return -errno;
>  			}

[PATCH v3 4/6] Fix read / write data offsets in read / write loops

[Michael Sartain]

The tot variable in __do_write and do_read is incremented with the amount read
/ written, but subsequent times through the loop the calls continue to use the
original data pointer.

Signed-off-by: Michael Sartain <mikesart@fastmail.com>
---
 trace-cmd-local.h | 2 +-
 trace-input.c     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/trace-cmd-local.h b/trace-cmd-local.h
index 8595a8a..b8ab35b 100644
--- a/trace-cmd-local.h
+++ b/trace-cmd-local.h
@@ -31,7 +31,7 @@ static ssize_t __do_write(int fd, const void *data, size_t size)
 	ssize_t w;
 
 	do {
-		w = TEMP_FAILURE_RETRY(write(fd, data, size - tot));
+		w = TEMP_FAILURE_RETRY(write(fd, data + tot, size - tot));
 		tot += w;
 
 		if (!w)
diff --git a/trace-input.c b/trace-input.c
index 251d32b..8395917 100644
--- a/trace-input.c
+++ b/trace-input.c
@@ -202,7 +202,7 @@ static ssize_t do_read(struct tracecmd_input *handle, void *data, size_t size)
 	ssize_t r;
 
 	do {
-		r = TEMP_FAILURE_RETRY(read(handle->fd, data, size - tot));
+		r = TEMP_FAILURE_RETRY(read(handle->fd, data + tot, size - tot));
 		tot += r;
 
 		if (!r)
-- 
2.11.0

Re: [PATCH v3 4/6] Fix read / write data offsets in read / write loops

[Steven Rostedt]

On Wed, 14 Jun 2017 18:27:59 -0600
Michael Sartain <mikesart@fastmail.com> wrote:

> The tot variable in __do_write and do_read is incremented with the amount read
> / written, but subsequent times through the loop the calls continue to use the
> original data pointer.
> 
> Signed-off-by: Michael Sartain <mikesart@fastmail.com>
> ---
>  trace-cmd-local.h | 2 +-
>  trace-input.c     | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/trace-cmd-local.h b/trace-cmd-local.h
> index 8595a8a..b8ab35b 100644
> --- a/trace-cmd-local.h
> +++ b/trace-cmd-local.h
> @@ -31,7 +31,7 @@ static ssize_t __do_write(int fd, const void *data, size_t size)
>  	ssize_t w;
>  
>  	do {
> -		w = TEMP_FAILURE_RETRY(write(fd, data, size - tot));
> +		w = TEMP_FAILURE_RETRY(write(fd, data + tot, size - tot));

Good catch. I'm going to modify this to remove the TEMP_FAILURE_RETRY()
though. I'll hopefully get this pushed out later today, and we could
add the write_intr() and friends later.

-- Steve

>  		tot += w;
>  
>  		if (!w)
> diff --git a/trace-input.c b/trace-input.c
> index 251d32b..8395917 100644
> --- a/trace-input.c
> +++ b/trace-input.c
> @@ -202,7 +202,7 @@ static ssize_t do_read(struct tracecmd_input *handle, void *data, size_t size)
>  	ssize_t r;
>  
>  	do {
> -		r = TEMP_FAILURE_RETRY(read(handle->fd, data, size - tot));
> +		r = TEMP_FAILURE_RETRY(read(handle->fd, data + tot, size - tot));
>  		tot += r;
>  
>  		if (!r)

Re: [PATCH v3 4/6] Fix read / write data offsets in read / write loops

[Michael Sartain]

On Wed, Jun 21, 2017 at 09:29:14AM -0400, Steven Rostedt wrote:
> On Wed, 14 Jun 2017 18:27:59 -0600
> Michael Sartain <mikesart@fastmail.com> wrote:
> 
> > The tot variable in __do_write and do_read is incremented with the amount read
> > / written, but subsequent times through the loop the calls continue to use the
> > original data pointer.
> > 
> > Signed-off-by: Michael Sartain <mikesart@fastmail.com>
> > ---
> >  trace-cmd-local.h | 2 +-
> >  trace-input.c     | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/trace-cmd-local.h b/trace-cmd-local.h
> > index 8595a8a..b8ab35b 100644
> > --- a/trace-cmd-local.h
> > +++ b/trace-cmd-local.h
> > @@ -31,7 +31,7 @@ static ssize_t __do_write(int fd, const void *data, size_t size)
> >  	ssize_t w;
> >  
> >  	do {
> > -		w = TEMP_FAILURE_RETRY(write(fd, data, size - tot));
> > +		w = TEMP_FAILURE_RETRY(write(fd, data + tot, size - tot));
> 
> Good catch. I'm going to modify this to remove the TEMP_FAILURE_RETRY()
> though. I'll hopefully get this pushed out later today, and we could
> add the write_intr() and friends later.

I've got a couple other patches ready to submit. I'll wait for your push,
rebase those, and add a TEMP_FAILURE_RETRY -> *_intr() patch with those.

Thanks much Steve.
 -Mike

[PATCH v3 5/6] Fix function prototypes for __vwarning, __vpr_stat, and __vdie

[Michael Sartain]

They were declared:
  (const char *fmt, ...)
but implemented as:
  (const char *fmt, va_list ap)

Signed-off-by: Michael Sartain <mikesart@fastmail.com>
---
 event-utils.h | 4 ++--
 parse-utils.c | 2 ++
 trace-local.h | 2 +-
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/event-utils.h b/event-utils.h
index d1dc217..f709db2 100644
--- a/event-utils.h
+++ b/event-utils.h
@@ -31,8 +31,8 @@ void vpr_stat(const char *fmt, va_list ap);
 void __warning(const char *fmt, ...);
 void __pr_stat(const char *fmt, ...);
 
-void __vwarning(const char *fmt, ...);
-void __vpr_stat(const char *fmt, ...);
+void __vwarning(const char *fmt, va_list ap);
+void __vpr_stat(const char *fmt, va_list ap);
 
 #define min(x, y) ({				\
 	typeof(x) _min1 = (x);			\
diff --git a/parse-utils.c b/parse-utils.c
index 42c1885..5cc9688 100644
--- a/parse-utils.c
+++ b/parse-utils.c
@@ -23,6 +23,8 @@
 #include <stdarg.h>
 #include <errno.h>
 
+#include "event-utils.h"
+
 #define __weak __attribute__((weak))
 
 void __vwarning(const char *fmt, va_list ap)
diff --git a/trace-local.h b/trace-local.h
index fa987bc..fb9f599 100644
--- a/trace-local.h
+++ b/trace-local.h
@@ -198,6 +198,6 @@ int count_cpus(void);
 void die(const char *fmt, ...); /* Can be overriden */
 void *malloc_or_die(unsigned int size); /* Can be overridden */
 void __die(const char *fmt, ...);
-void __vdie(const char *fmt, ...);
+void __vdie(const char *fmt, va_list ap);
 
 #endif /* __TRACE_LOCAL_H */
-- 
2.11.0

[PATCH v3 6/6] Fix cases where string literals were passed as string format args

[Michael Sartain]

Signed-off-by: Michael Sartain <mikesart@fastmail.com>
---
 trace-capture.c | 12 ++++++------
 trace-dialog.c  |  4 ++--
 trace-filter.c  | 10 +++++-----
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/trace-capture.c b/trace-capture.c
index 1acfe44..4e1392e 100644
--- a/trace-capture.c
+++ b/trace-capture.c
@@ -1019,7 +1019,7 @@ static void save_events(struct trace_capture *cap,
 	tracecmd_xml_write_element(handle, "CaptureType", "Events");
 
 	for (i = 0; systems && systems[i]; i++)
-		tracecmd_xml_write_element(handle, "System", systems[i]);
+		tracecmd_xml_write_element(handle, "System", "%s", systems[i]);
 
 	if (!events || events[0] < 0)
 		return;
@@ -1029,8 +1029,8 @@ static void save_events(struct trace_capture *cap,
 		event = pevent_find_event(pevent, events[i]);
 		if (event) {
 			tracecmd_xml_start_sub_system(handle, "Event");
-			tracecmd_xml_write_element(handle, "System", event->system);
-			tracecmd_xml_write_element(handle, "Name", event->name);
+			tracecmd_xml_write_element(handle, "System", "%s", event->system);
+			tracecmd_xml_write_element(handle, "Name", "%s", event->name);
 			tracecmd_xml_end_sub_system(handle);
 		}
 	}
@@ -1068,15 +1068,15 @@ static void save_settings(struct trace_capture *cap, const char *filename)
 
 	update_plugin(cap);
 	if (info->cap_plugin)
-		tracecmd_xml_write_element(handle, "Plugin", info->cap_plugin);
+		tracecmd_xml_write_element(handle, "Plugin", "%s", info->cap_plugin);
 
 	command = gtk_entry_get_text(GTK_ENTRY(cap->command_entry));
 	if (command && strlen(command) && !is_just_ws(command))
-		tracecmd_xml_write_element(handle, "Command", command);
+		tracecmd_xml_write_element(handle, "Command", "%s", command);
 
 	file = gtk_entry_get_text(GTK_ENTRY(cap->file_entry));
 	if (file && strlen(file) && !is_just_ws(file))
-		tracecmd_xml_write_element(handle, "File", file);
+		tracecmd_xml_write_element(handle, "File", "%s", file);
 
 	tracecmd_xml_end_system(handle);
 
diff --git a/trace-dialog.c b/trace-dialog.c
index 5d3aabe..b5776cc 100644
--- a/trace-dialog.c
+++ b/trace-dialog.c
@@ -218,7 +218,7 @@ void warning(const char *fmt, ...)
 	errno = 0;
 
 	trace_dialog(GTK_WINDOW(parent_window), TRACE_GUI_WARNING,
-		     str->str);
+		     "%s", str->str);
 
 	g_string_free(str, TRUE);
 }
@@ -425,7 +425,7 @@ void trace_show_record_dialog(GtkWindow *parent, struct pevent *pevent,
 
 	if (s.buffer_size) {
 		trace_seq_terminate(&s);
-		trace_dialog(parent, TRACE_GUI_OTHER, s.buffer);
+		trace_dialog(parent, TRACE_GUI_OTHER, "%s", s.buffer);
 	}
 
 	trace_seq_destroy(&s);
diff --git a/trace-filter.c b/trace-filter.c
index 1c36d84..bd8cb11 100644
--- a/trace-filter.c
+++ b/trace-filter.c
@@ -2043,7 +2043,7 @@ int trace_filter_save_events(struct tracecmd_xml_handle *handle,
 					     &event_ids);
 
 	for (i = 0; systems && systems[i]; i++)
-		tracecmd_xml_write_element(handle, "System", systems[i]);
+		tracecmd_xml_write_element(handle, "System", "%s", systems[i]);
 
 	for (i = 0; event_ids && event_ids[i] > 0; i++) {
 		str = pevent_filter_make_string(filter, event_ids[i]);
@@ -2060,11 +2060,11 @@ int trace_filter_save_events(struct tracecmd_xml_handle *handle,
 			}
 
 			tracecmd_xml_start_sub_system(handle, "Event");
-			tracecmd_xml_write_element(handle, "System", event->system);
-			tracecmd_xml_write_element(handle, "Name", event->name);
+			tracecmd_xml_write_element(handle, "System", "%s", event->system);
+			tracecmd_xml_write_element(handle, "Name", "%s", event->name);
 			/* If this is has an advanced filter, include that too */
 			if (strcmp(str, "TRUE") != 0) {
-				tracecmd_xml_write_element(handle, "Advanced",
+				tracecmd_xml_write_element(handle, "Advanced", "%s",
 							   str);
 			}
 			tracecmd_xml_end_sub_system(handle);
@@ -2088,7 +2088,7 @@ int trace_filter_save_tasks(struct tracecmd_xml_handle *handle,
 
 	for (i = 0; pids[i] >= 0; i++) {
 		snprintf(buffer, 100, "%d", pids[i]);
-		tracecmd_xml_write_element(handle, "Task", buffer);
+		tracecmd_xml_write_element(handle, "Task", "%s", buffer);
 	}
 
 	free(pids);
-- 
2.11.0