[tip:perf/core] tools lib traceevent: Add UL suffix to MISSING_EVENTS
Commit-ID: 6d36ce261614fbac3557cc58ba6a33424944c8a2 Gitweb: https://git.kernel.org/tip/6d36ce261614fbac3557cc58ba6a33424944c8a2 Author: Michael Sartain <mikes...@fastmail.com> AuthorDate: Thu, 11 Jan 2018 19:47:49 -0500 Committer: Arnaldo Carvalho de Melo <a...@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 <mikes...@fastmail.com> Acked-by: Namhyung Kim <namhy...@kernel.org> Cc: Andrew Morton <a...@linux-foundation.org> Link: http://lkml.kernel.org/r/20171016165542.13038-4-mikes...@fastmail.com Link: http://lkml.kernel.org/r/20180112004822.829533...@goodmis.org Signed-off-by: Steven Rostedt <rost...@goodmis.org> Signed-off-by: Arnaldo Carvalho de Melo <a...@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)
[tip:perf/core] tools lib traceevent: Add UL suffix to MISSING_EVENTS
Commit-ID: 6d36ce261614fbac3557cc58ba6a33424944c8a2 Gitweb: https://git.kernel.org/tip/6d36ce261614fbac3557cc58ba6a33424944c8a2 Author: Michael Sartain AuthorDate: Thu, 11 Jan 2018 19:47:49 -0500 Committer: Arnaldo Carvalho de Melo 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 Acked-by: Namhyung Kim Cc: Andrew Morton Link: http://lkml.kernel.org/r/20171016165542.13038-4-mikes...@fastmail.com Link: http://lkml.kernel.org/r/20180112004822.829533...@goodmis.org Signed-off-by: Steven Rostedt Signed-off-by: Arnaldo Carvalho de Melo --- 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)
[tip:perf/core] tools lib traceevent: Fix bad force_token escape sequence
Commit-ID: 952a99ccfa9db2f9a32810fc9c0084f532dd871a Gitweb: https://git.kernel.org/tip/952a99ccfa9db2f9a32810fc9c0084f532dd871a Author: Michael Sartain <mikes...@fastmail.com> AuthorDate: Thu, 11 Jan 2018 19:47:42 -0500 Committer: Arnaldo Carvalho de Melo <a...@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 <mikes...@fastmail.com> Acked-by: Namhyung Kim <namhy...@kernel.org> Cc: Andrew Morton <a...@linux-foundation.org> Link: http://lkml.kernel.org/r/20180112004821.827168...@goodmis.org Link: http://lkml.kernel.org/r/d32d37c10ce0912851e1fb78d1e0c946bcd9.1497486273.git.mikes...@fastmail.com Signed-off-by: Steven Rostedt <rost...@goodmis.org> Signed-off-by: Arnaldo Carvalho de Melo <a...@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;
[tip:perf/core] tools lib traceevent: Fix bad force_token escape sequence
Commit-ID: 952a99ccfa9db2f9a32810fc9c0084f532dd871a Gitweb: https://git.kernel.org/tip/952a99ccfa9db2f9a32810fc9c0084f532dd871a Author: Michael Sartain AuthorDate: Thu, 11 Jan 2018 19:47:42 -0500 Committer: Arnaldo Carvalho de Melo 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 Acked-by: Namhyung Kim Cc: Andrew Morton Link: http://lkml.kernel.org/r/20180112004821.827168...@goodmis.org Link: http://lkml.kernel.org/r/d32d37c10ce0912851e1fb78d1e0c946bcd9.1497486273.git.mikes...@fastmail.com Signed-off-by: Steven Rostedt Signed-off-by: Arnaldo Carvalho de Melo --- 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 v2 2/4] trace-cmd: Fix NULL pointer being passed to memcpy
Signed-off-by: Michael Sartain <mikes...@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(>list, >options); return option; -- 2.14.2
[PATCH v2 2/4] trace-cmd: Fix NULL pointer being passed to memcpy
Signed-off-by: Michael Sartain --- 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(>list, >options); return option; -- 2.14.2
[PATCH v2 0/4] trace-cmd: Fixes for four minor bugs
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
Signed-off-by: Michael Sartain <mikes...@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 0/4] trace-cmd: Fixes for four minor bugs
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
Signed-off-by: Michael Sartain --- 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 3/4] trace-cmd: Add UL suffix to MISSING_EVENTS since ints shouldn't be left shifted by 31
Signed-off-by: Michael Sartain <mikes...@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 3/4] trace-cmd: Add UL suffix to MISSING_EVENTS since ints shouldn't be left shifted by 31
Signed-off-by: Michael Sartain --- 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
Signed-off-by: Michael Sartain <mikes...@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 v2 4/4] trace-cmd: Use unsigned values in Hsieh's trace_hash fast hash function
Signed-off-by: Michael Sartain --- 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
Re: [PATCH 2/5] trace-cmd: Fix NULL pointer being passed to memcpy
On Mon, Oct 09, 2017 at 06:24:32PM -0400, Steven Rostedt wrote: > On Sat, 12 Aug 2017 11:30:44 -0600 > Michael Sartain <mikes...@fastmail.com> wrote: > > > Signed-off-by: Michael Sartain <mikes...@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..84b21b0 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 NULL data */ > > + if (data) > > + memcpy(option->data, data, size); > > Is this a problem, as when this happens, size should be zero. Does it > crash with data=NULL and size=0, or have you seen size not be zero? I got an ASAN warning, but you are correct - the size was 0 and it did not crash.
Re: [PATCH 2/5] trace-cmd: Fix NULL pointer being passed to memcpy
On Mon, Oct 09, 2017 at 06:24:32PM -0400, Steven Rostedt wrote: > On Sat, 12 Aug 2017 11:30:44 -0600 > Michael Sartain wrote: > > > Signed-off-by: Michael Sartain > > --- > > trace-output.c | 6 +- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/trace-output.c b/trace-output.c > > index bfe6331..84b21b0 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 NULL data */ > > + if (data) > > + memcpy(option->data, data, size); > > Is this a problem, as when this happens, size should be zero. Does it > crash with data=NULL and size=0, or have you seen size not be zero? I got an ASAN warning, but you are correct - the size was 0 and it did not crash.
Re: [PATCH 0/5] trace-cmd: Fixes for four small bugs plus minor cleanup
Ping? Were there any concerns with these or things needed to be done before they could be merged? Thanks. On Sat, Aug 12, 2017, at 11:30 AM, Michael Sartain wrote: > Thanks much. > -Mike > > --- > > Michael Sartain (5): > trace-cmd: Fix incorrect malloc size arg: *item instead of item > trace-cmd: Fix NULL pointer being passed to memcpy > trace-cmd: Add ULL 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 > trace-cmd: Remove unused view_width variable > > kbuffer-parse.c| 4 ++-- > trace-dialog.c | 2 +- > trace-graph.c | 2 -- > trace-hash-local.h | 4 ++-- > trace-output.c | 6 +- > 5 files changed, 10 insertions(+), 8 deletions(-) > > -- > 2.13.2 >
Re: [PATCH 0/5] trace-cmd: Fixes for four small bugs plus minor cleanup
Ping? Were there any concerns with these or things needed to be done before they could be merged? Thanks. On Sat, Aug 12, 2017, at 11:30 AM, Michael Sartain wrote: > Thanks much. > -Mike > > --- > > Michael Sartain (5): > trace-cmd: Fix incorrect malloc size arg: *item instead of item > trace-cmd: Fix NULL pointer being passed to memcpy > trace-cmd: Add ULL 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 > trace-cmd: Remove unused view_width variable > > kbuffer-parse.c| 4 ++-- > trace-dialog.c | 2 +- > trace-graph.c | 2 -- > trace-hash-local.h | 4 ++-- > trace-output.c | 6 +- > 5 files changed, 10 insertions(+), 8 deletions(-) > > -- > 2.13.2 >
[PATCH 4/5] trace-cmd: Use unsigned values in Hsieh's trace_hash fast hash function
Signed int values were being used where the original code used uint32_t types: http://www.azillionmonkeys.com/qed/hash.html Right shifting negative int values has implementation-defined and left shifting has undefined behavior. On my platform (x86_64) right shifting was doing sign extension and filling high bits with 1s, which is different than the original algorithm. Signed-off-by: Michael Sartain <mikes...@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..b3f9b06 100644 --- a/trace-hash-local.h +++ b/trace-hash-local.h @@ -22,7 +22,7 @@ static inline unsigned int trace_hash(int val) { - int hash, tmp; + unsigned int hash, tmp; hash = 12546869;/* random prime */ @@ -34,7 +34,7 @@ static inline unsigned int trace_hash(int val) */ hash += (val & 0x); - tmp = (val >> 16) ^ hash; + tmp = ((unsigned int)val >> 16) ^ hash; hash = (hash << 16) ^ tmp; hash += hash >> 11; -- 2.13.2
[PATCH 4/5] trace-cmd: Use unsigned values in Hsieh's trace_hash fast hash function
Signed int values were being used where the original code used uint32_t types: http://www.azillionmonkeys.com/qed/hash.html Right shifting negative int values has implementation-defined and left shifting has undefined behavior. On my platform (x86_64) right shifting was doing sign extension and filling high bits with 1s, which is different than the original algorithm. Signed-off-by: Michael Sartain --- 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..b3f9b06 100644 --- a/trace-hash-local.h +++ b/trace-hash-local.h @@ -22,7 +22,7 @@ static inline unsigned int trace_hash(int val) { - int hash, tmp; + unsigned int hash, tmp; hash = 12546869;/* random prime */ @@ -34,7 +34,7 @@ static inline unsigned int trace_hash(int val) */ hash += (val & 0x); - tmp = (val >> 16) ^ hash; + tmp = ((unsigned int)val >> 16) ^ hash; hash = (hash << 16) ^ tmp; hash += hash >> 11; -- 2.13.2
[PATCH 1/5] trace-cmd: Fix incorrect malloc size arg: *item instead of item
Signed-off-by: Michael Sartain <mikes...@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.13.2
[PATCH 1/5] trace-cmd: Fix incorrect malloc size arg: *item instead of item
Signed-off-by: Michael Sartain --- 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.13.2
[PATCH 3/5] trace-cmd: Add ULL suffix to MISSING_EVENTS since ints shouldn't be left shifted by 31
Signed-off-by: Michael Sartain <mikes...@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..dde642c 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 (1ULL << 31) +#define MISSING_STORED (1ULL << 30) #define COMMIT_MASK ((1 << 27) - 1) -- 2.13.2
[PATCH 3/5] trace-cmd: Add ULL suffix to MISSING_EVENTS since ints shouldn't be left shifted by 31
Signed-off-by: Michael Sartain --- kbuffer-parse.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kbuffer-parse.c b/kbuffer-parse.c index 4e6e95e..dde642c 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 (1ULL << 31) +#define MISSING_STORED (1ULL << 30) #define COMMIT_MASK ((1 << 27) - 1) -- 2.13.2
[PATCH 5/5] trace-cmd: Remove unused view_width variable
Signed-off-by: Michael Sartain <mikes...@fastmail.com> --- trace-graph.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/trace-graph.c b/trace-graph.c index 1db342f..2c49549 100644 --- a/trace-graph.c +++ b/trace-graph.c @@ -1263,7 +1263,6 @@ static void draw_info_box(struct graph_info *ginfo, const gchar *buffer, gint width, height; GdkPixmap *pix; static GdkGC *pix_bg; - gint view_width; gint view_start; if (!pix_bg) { @@ -1284,7 +1283,6 @@ static void draw_info_box(struct graph_info *ginfo, const gchar *buffer, height += PLOT_BOARDER * 2; view_start = gtk_adjustment_get_value(ginfo->hadj); - view_width = gtk_adjustment_get_page_size(ginfo->hadj); if (x > view_start + width) x -= width; -- 2.13.2
[PATCH 5/5] trace-cmd: Remove unused view_width variable
Signed-off-by: Michael Sartain --- trace-graph.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/trace-graph.c b/trace-graph.c index 1db342f..2c49549 100644 --- a/trace-graph.c +++ b/trace-graph.c @@ -1263,7 +1263,6 @@ static void draw_info_box(struct graph_info *ginfo, const gchar *buffer, gint width, height; GdkPixmap *pix; static GdkGC *pix_bg; - gint view_width; gint view_start; if (!pix_bg) { @@ -1284,7 +1283,6 @@ static void draw_info_box(struct graph_info *ginfo, const gchar *buffer, height += PLOT_BOARDER * 2; view_start = gtk_adjustment_get_value(ginfo->hadj); - view_width = gtk_adjustment_get_page_size(ginfo->hadj); if (x > view_start + width) x -= width; -- 2.13.2
[PATCH 0/5] trace-cmd: Fixes for four small bugs plus minor cleanup
Thanks much. -Mike --- Michael Sartain (5): trace-cmd: Fix incorrect malloc size arg: *item instead of item trace-cmd: Fix NULL pointer being passed to memcpy trace-cmd: Add ULL 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 trace-cmd: Remove unused view_width variable kbuffer-parse.c| 4 ++-- trace-dialog.c | 2 +- trace-graph.c | 2 -- trace-hash-local.h | 4 ++-- trace-output.c | 6 +- 5 files changed, 10 insertions(+), 8 deletions(-) -- 2.13.2
[PATCH 0/5] trace-cmd: Fixes for four small bugs plus minor cleanup
Thanks much. -Mike --- Michael Sartain (5): trace-cmd: Fix incorrect malloc size arg: *item instead of item trace-cmd: Fix NULL pointer being passed to memcpy trace-cmd: Add ULL 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 trace-cmd: Remove unused view_width variable kbuffer-parse.c| 4 ++-- trace-dialog.c | 2 +- trace-graph.c | 2 -- trace-hash-local.h | 4 ++-- trace-output.c | 6 +- 5 files changed, 10 insertions(+), 8 deletions(-) -- 2.13.2
[PATCH 2/5] trace-cmd: Fix NULL pointer being passed to memcpy
Signed-off-by: Michael Sartain <mikes...@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..84b21b0 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 NULL data */ + if (data) + memcpy(option->data, data, size); + list_add_tail(>list, >options); return option; -- 2.13.2
[PATCH 2/5] trace-cmd: Fix NULL pointer being passed to memcpy
Signed-off-by: Michael Sartain --- trace-output.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/trace-output.c b/trace-output.c index bfe6331..84b21b0 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 NULL data */ + if (data) + memcpy(option->data, data, size); + list_add_tail(>list, >options); return option; -- 2.13.2
[PATCH v2] tracing: Add saved_tgids file to show cached pid to tgid mappings
Export the cached pid / tgid mappings added by Joel Fernandes' patch [1] in debugfs tracing saved_tgids file. This allows user apps to translate the pids from a trace to their respective thread group. Example saved_tgids file with pid / tgid values separated by ' ': # cat saved_tgids 1048 1048 1047 1047 7 7 1049 1047 1054 1047 1053 1047 Let userspace apps reading binary buffer know tgid's. [1] https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1434654.html Reviewed-by: Joel Fernandes <joe...@google.com> Signed-off-by: Michael Sartain <mikes...@fastmail.com> --- [v2] Don't reuse cmdlines code for saved_tgids file. kernel/trace/trace.c | 73 1 file changed, 73 insertions(+) diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c index c72c36c..e962fb3 100644 --- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c @@ -4701,6 +4701,76 @@ static const struct file_operations tracing_readme_fops = { .llseek = generic_file_llseek, }; +static void *saved_tgids_next(struct seq_file *m, void *v, loff_t *pos) +{ + int *ptr = v; + + if (*pos || m->count) + ptr++; + + (*pos)++; + + for (; ptr <= _map[PID_MAX_DEFAULT]; ptr++) { + if (trace_find_tgid(*ptr)) + return ptr; + } + + return NULL; +} + +static void *saved_tgids_start(struct seq_file *m, loff_t *pos) +{ + void *v; + loff_t l = 0; + + if (!tgid_map) + return NULL; + + v = _map[0]; + while (l <= *pos) { + v = saved_tgids_next(m, v, ); + if (!v) + return NULL; + } + + return v; +} + +static void saved_tgids_stop(struct seq_file *m, void *v) +{ +} + +static int saved_tgids_show(struct seq_file *m, void *v) +{ + int pid = (int *)v - tgid_map; + + seq_printf(m, "%d %d\n", pid, trace_find_tgid(pid)); + return 0; +} + +static const struct seq_operations tracing_saved_tgids_seq_ops = { + .start = saved_tgids_start, + .stop = saved_tgids_stop, + .next = saved_tgids_next, + .show = saved_tgids_show, +}; + +static int tracing_saved_tgids_open(struct inode *inode, struct file *filp) +{ + if (tracing_disabled) + return -ENODEV; + + return seq_open(filp, _saved_tgids_seq_ops); +} + + +static const struct file_operations tracing_saved_tgids_fops = { + .open = tracing_saved_tgids_open, + .read = seq_read, + .llseek = seq_lseek, + .release= seq_release, +}; + static void *saved_cmdlines_next(struct seq_file *m, void *v, loff_t *pos) { unsigned int *ptr = v; @@ -7945,6 +8015,9 @@ static __init int tracer_init_tracefs(void) trace_create_file("saved_cmdlines_size", 0644, d_tracer, NULL, _saved_cmdlines_size_fops); + trace_create_file("saved_tgids", 0444, d_tracer, + NULL, _saved_tgids_fops); + trace_eval_init(); trace_create_eval_file(d_tracer); -- 2.13.2
[PATCH v2] tracing: Add saved_tgids file to show cached pid to tgid mappings
Export the cached pid / tgid mappings added by Joel Fernandes' patch [1] in debugfs tracing saved_tgids file. This allows user apps to translate the pids from a trace to their respective thread group. Example saved_tgids file with pid / tgid values separated by ' ': # cat saved_tgids 1048 1048 1047 1047 7 7 1049 1047 1054 1047 1053 1047 Let userspace apps reading binary buffer know tgid's. [1] https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1434654.html Reviewed-by: Joel Fernandes Signed-off-by: Michael Sartain --- [v2] Don't reuse cmdlines code for saved_tgids file. kernel/trace/trace.c | 73 1 file changed, 73 insertions(+) diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c index c72c36c..e962fb3 100644 --- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c @@ -4701,6 +4701,76 @@ static const struct file_operations tracing_readme_fops = { .llseek = generic_file_llseek, }; +static void *saved_tgids_next(struct seq_file *m, void *v, loff_t *pos) +{ + int *ptr = v; + + if (*pos || m->count) + ptr++; + + (*pos)++; + + for (; ptr <= _map[PID_MAX_DEFAULT]; ptr++) { + if (trace_find_tgid(*ptr)) + return ptr; + } + + return NULL; +} + +static void *saved_tgids_start(struct seq_file *m, loff_t *pos) +{ + void *v; + loff_t l = 0; + + if (!tgid_map) + return NULL; + + v = _map[0]; + while (l <= *pos) { + v = saved_tgids_next(m, v, ); + if (!v) + return NULL; + } + + return v; +} + +static void saved_tgids_stop(struct seq_file *m, void *v) +{ +} + +static int saved_tgids_show(struct seq_file *m, void *v) +{ + int pid = (int *)v - tgid_map; + + seq_printf(m, "%d %d\n", pid, trace_find_tgid(pid)); + return 0; +} + +static const struct seq_operations tracing_saved_tgids_seq_ops = { + .start = saved_tgids_start, + .stop = saved_tgids_stop, + .next = saved_tgids_next, + .show = saved_tgids_show, +}; + +static int tracing_saved_tgids_open(struct inode *inode, struct file *filp) +{ + if (tracing_disabled) + return -ENODEV; + + return seq_open(filp, _saved_tgids_seq_ops); +} + + +static const struct file_operations tracing_saved_tgids_fops = { + .open = tracing_saved_tgids_open, + .read = seq_read, + .llseek = seq_lseek, + .release= seq_release, +}; + static void *saved_cmdlines_next(struct seq_file *m, void *v, loff_t *pos) { unsigned int *ptr = v; @@ -7945,6 +8015,9 @@ static __init int tracer_init_tracefs(void) trace_create_file("saved_cmdlines_size", 0644, d_tracer, NULL, _saved_cmdlines_size_fops); + trace_create_file("saved_tgids", 0444, d_tracer, + NULL, _saved_tgids_fops); + trace_eval_init(); trace_create_eval_file(d_tracer); -- 2.13.2
Re: [PATCH] tracing: Add saved_tgids file to show cached pid to tgid mappings
On Fri, Jun 30, 2017 at 05:50:57PM -0400, Steven Rostedt wrote: > On Fri, 30 Jun 2017 11:17:50 -0600 > Michael Sartain <mikes...@fastmail.com> wrote: > > > Export the cached pid / tgid mappings to userspace. This allows user > > apps to translate the pids from a trace to their respective thread > > group. > > > > Example saved_tgids file with pid / tgid values separated by ' ': > > > > # cat saved_tgids > > 1048 1048 > > 1047 1047 > > 7 7 > > 1049 1047 > > 1054 1047 > > 1053 1047 > > > > [ Impact: let userspace apps reading binary buffer know tgid's ] > > Impact line? Linus put a kibosh on this a while ago... > > https://lkml.org/lkml/2009/4/15/296 > > Although, I will admit that this line is actually useful. Just remove > the brackets. Ah, thank you. I saw it used in the previous cmdline patch and used it as a reference. > > Signed-off-by: Michael Sartain <mikes...@fastmail.com> > > --- > > kernel/trace/trace.c | 60 > > +++- > > 1 file changed, 59 insertions(+), 1 deletion(-) > > > > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c > > index 68c214b..ca84c97 100644 > > --- a/kernel/trace/trace.c > > +++ b/kernel/trace/trace.c > > @@ -4692,6 +4692,7 @@ static const struct file_operations > > tracing_readme_fops = { > > static void *saved_cmdlines_next(struct seq_file *m, void *v, loff_t *pos) > > { > > unsigned int *ptr = v; > > + long tgid_check = (long) m->private; > > I really don't like the subtle use of having m->private != NULL mean > this is for tgid listing. > > In fact, I don't see the purpose of reusing the seq code. The cmdlines > and tgid map are quite different. Just create its own functions. I > don't see the benefit of trying to reuse this except for making the > code more complex. Will do. Joel was also kind enough to send me feedback about RFCs and merge windows, etc. so I apologize - wasn't trying to get anything by anyone, just that this is my first real patch and I've obviously got a lot to learn here. Thank you for the comments. I'll fix and resubmit next week. -Mike
Re: [PATCH] tracing: Add saved_tgids file to show cached pid to tgid mappings
On Fri, Jun 30, 2017 at 05:50:57PM -0400, Steven Rostedt wrote: > On Fri, 30 Jun 2017 11:17:50 -0600 > Michael Sartain wrote: > > > Export the cached pid / tgid mappings to userspace. This allows user > > apps to translate the pids from a trace to their respective thread > > group. > > > > Example saved_tgids file with pid / tgid values separated by ' ': > > > > # cat saved_tgids > > 1048 1048 > > 1047 1047 > > 7 7 > > 1049 1047 > > 1054 1047 > > 1053 1047 > > > > [ Impact: let userspace apps reading binary buffer know tgid's ] > > Impact line? Linus put a kibosh on this a while ago... > > https://lkml.org/lkml/2009/4/15/296 > > Although, I will admit that this line is actually useful. Just remove > the brackets. Ah, thank you. I saw it used in the previous cmdline patch and used it as a reference. > > Signed-off-by: Michael Sartain > > --- > > kernel/trace/trace.c | 60 > > +++- > > 1 file changed, 59 insertions(+), 1 deletion(-) > > > > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c > > index 68c214b..ca84c97 100644 > > --- a/kernel/trace/trace.c > > +++ b/kernel/trace/trace.c > > @@ -4692,6 +4692,7 @@ static const struct file_operations > > tracing_readme_fops = { > > static void *saved_cmdlines_next(struct seq_file *m, void *v, loff_t *pos) > > { > > unsigned int *ptr = v; > > + long tgid_check = (long) m->private; > > I really don't like the subtle use of having m->private != NULL mean > this is for tgid listing. > > In fact, I don't see the purpose of reusing the seq code. The cmdlines > and tgid map are quite different. Just create its own functions. I > don't see the benefit of trying to reuse this except for making the > code more complex. Will do. Joel was also kind enough to send me feedback about RFCs and merge windows, etc. so I apologize - wasn't trying to get anything by anyone, just that this is my first real patch and I've obviously got a lot to learn here. Thank you for the comments. I'll fix and resubmit next week. -Mike
[PATCH] tracing: Add saved_tgids file to show cached pid to tgid mappings
Export the cached pid / tgid mappings to userspace. This allows user apps to translate the pids from a trace to their respective thread group. Example saved_tgids file with pid / tgid values separated by ' ': # cat saved_tgids 1048 1048 1047 1047 7 7 1049 1047 1054 1047 1053 1047 [ Impact: let userspace apps reading binary buffer know tgid's ] Signed-off-by: Michael Sartain <mikes...@fastmail.com> --- kernel/trace/trace.c | 60 +++- 1 file changed, 59 insertions(+), 1 deletion(-) diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c index 68c214b..ca84c97 100644 --- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c @@ -4692,6 +4692,7 @@ static const struct file_operations tracing_readme_fops = { static void *saved_cmdlines_next(struct seq_file *m, void *v, loff_t *pos) { unsigned int *ptr = v; + long tgid_check = (long) m->private; if (*pos || m->count) ptr++; @@ -4702,6 +4703,8 @@ static void *saved_cmdlines_next(struct seq_file *m, void *v, loff_t *pos) ptr++) { if (*ptr == -1 || *ptr == NO_CMDLINE_MAP) continue; + if (tgid_check && !trace_find_tgid(*ptr)) + continue; return ptr; } @@ -4713,6 +4716,10 @@ static void *saved_cmdlines_start(struct seq_file *m, loff_t *pos) { void *v; loff_t l = 0; + long tgid_check = (long) m->private; + + if (tgid_check && !tgid_map) + return NULL; preempt_disable(); arch_spin_lock(_cmdline_lock); @@ -4743,6 +4750,14 @@ static int saved_cmdlines_show(struct seq_file *m, void *v) return 0; } +static int saved_tgids_show(struct seq_file *m, void *v) +{ + unsigned int *pid = v; + + seq_printf(m, "%d %d\n", *pid, trace_find_tgid(*pid)); + return 0; +} + static const struct seq_operations tracing_saved_cmdlines_seq_ops = { .start = saved_cmdlines_start, .next = saved_cmdlines_next, @@ -4750,12 +4765,45 @@ static const struct seq_operations tracing_saved_cmdlines_seq_ops = { .show = saved_cmdlines_show, }; +static const struct seq_operations tracing_saved_tgids_seq_ops = { + .start = saved_cmdlines_start, + .next = saved_cmdlines_next, + .stop = saved_cmdlines_stop, + .show = saved_tgids_show, +}; + static int tracing_saved_cmdlines_open(struct inode *inode, struct file *filp) { + int ret; + + if (tracing_disabled) + return -ENODEV; + + ret = seq_open(filp, _saved_cmdlines_seq_ops); + if (!ret) { + struct seq_file *m = filp->private_data; + + m->private = (void *) 0; /* Do not check for valid tgids */ + } + + return ret; +} + +static int tracing_saved_tgids_open(struct inode *inode, struct file *filp) +{ + int ret; + if (tracing_disabled) return -ENODEV; - return seq_open(filp, _saved_cmdlines_seq_ops); + ret = seq_open(filp, _saved_tgids_seq_ops); + if (!ret) { + struct seq_file *m = filp->private_data; + + m->private = (void *) 1; /* Check for valid tgids */ + } + + return ret; } static const struct file_operations tracing_saved_cmdlines_fops = { @@ -4765,6 +4813,13 @@ static const struct file_operations tracing_saved_cmdlines_fops = { .release= seq_release, }; +static const struct file_operations tracing_saved_tgids_fops = { + .open = tracing_saved_tgids_open, + .read = seq_read, + .llseek = seq_lseek, + .release= seq_release, +}; + static ssize_t tracing_saved_cmdlines_size_read(struct file *filp, char __user *ubuf, size_t cnt, loff_t *ppos) @@ -7933,6 +7988,9 @@ static __init int tracer_init_tracefs(void) trace_create_file("saved_cmdlines_size", 0644, d_tracer, NULL, _saved_cmdlines_size_fops); + trace_create_file("saved_tgids", 0444, d_tracer, + NULL, _saved_tgids_fops); + trace_eval_init(); trace_create_eval_file(d_tracer); -- 2.11.0
[PATCH] tracing: Add saved_tgids file to show cached pid to tgid mappings
Export the cached pid / tgid mappings to userspace. This allows user apps to translate the pids from a trace to their respective thread group. Example saved_tgids file with pid / tgid values separated by ' ': # cat saved_tgids 1048 1048 1047 1047 7 7 1049 1047 1054 1047 1053 1047 [ Impact: let userspace apps reading binary buffer know tgid's ] Signed-off-by: Michael Sartain --- kernel/trace/trace.c | 60 +++- 1 file changed, 59 insertions(+), 1 deletion(-) diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c index 68c214b..ca84c97 100644 --- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c @@ -4692,6 +4692,7 @@ static const struct file_operations tracing_readme_fops = { static void *saved_cmdlines_next(struct seq_file *m, void *v, loff_t *pos) { unsigned int *ptr = v; + long tgid_check = (long) m->private; if (*pos || m->count) ptr++; @@ -4702,6 +4703,8 @@ static void *saved_cmdlines_next(struct seq_file *m, void *v, loff_t *pos) ptr++) { if (*ptr == -1 || *ptr == NO_CMDLINE_MAP) continue; + if (tgid_check && !trace_find_tgid(*ptr)) + continue; return ptr; } @@ -4713,6 +4716,10 @@ static void *saved_cmdlines_start(struct seq_file *m, loff_t *pos) { void *v; loff_t l = 0; + long tgid_check = (long) m->private; + + if (tgid_check && !tgid_map) + return NULL; preempt_disable(); arch_spin_lock(_cmdline_lock); @@ -4743,6 +4750,14 @@ static int saved_cmdlines_show(struct seq_file *m, void *v) return 0; } +static int saved_tgids_show(struct seq_file *m, void *v) +{ + unsigned int *pid = v; + + seq_printf(m, "%d %d\n", *pid, trace_find_tgid(*pid)); + return 0; +} + static const struct seq_operations tracing_saved_cmdlines_seq_ops = { .start = saved_cmdlines_start, .next = saved_cmdlines_next, @@ -4750,12 +4765,45 @@ static const struct seq_operations tracing_saved_cmdlines_seq_ops = { .show = saved_cmdlines_show, }; +static const struct seq_operations tracing_saved_tgids_seq_ops = { + .start = saved_cmdlines_start, + .next = saved_cmdlines_next, + .stop = saved_cmdlines_stop, + .show = saved_tgids_show, +}; + static int tracing_saved_cmdlines_open(struct inode *inode, struct file *filp) { + int ret; + + if (tracing_disabled) + return -ENODEV; + + ret = seq_open(filp, _saved_cmdlines_seq_ops); + if (!ret) { + struct seq_file *m = filp->private_data; + + m->private = (void *) 0; /* Do not check for valid tgids */ + } + + return ret; +} + +static int tracing_saved_tgids_open(struct inode *inode, struct file *filp) +{ + int ret; + if (tracing_disabled) return -ENODEV; - return seq_open(filp, _saved_cmdlines_seq_ops); + ret = seq_open(filp, _saved_tgids_seq_ops); + if (!ret) { + struct seq_file *m = filp->private_data; + + m->private = (void *) 1; /* Check for valid tgids */ + } + + return ret; } static const struct file_operations tracing_saved_cmdlines_fops = { @@ -4765,6 +4813,13 @@ static const struct file_operations tracing_saved_cmdlines_fops = { .release= seq_release, }; +static const struct file_operations tracing_saved_tgids_fops = { + .open = tracing_saved_tgids_open, + .read = seq_read, + .llseek = seq_lseek, + .release= seq_release, +}; + static ssize_t tracing_saved_cmdlines_size_read(struct file *filp, char __user *ubuf, size_t cnt, loff_t *ppos) @@ -7933,6 +7988,9 @@ static __init int tracer_init_tracefs(void) trace_create_file("saved_cmdlines_size", 0644, d_tracer, NULL, _saved_cmdlines_size_fops); + trace_create_file("saved_tgids", 0444, d_tracer, + NULL, _saved_tgids_fops); + trace_eval_init(); trace_create_eval_file(d_tracer); -- 2.11.0
Re: [PATCH v3 4/6] Fix read / write data offsets in read / write loops
On Wed, Jun 21, 2017 at 09:29:14AM -0400, Steven Rostedt wrote: > On Wed, 14 Jun 2017 18:27:59 -0600 > Michael Sartain <mikes...@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 <mikes...@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
Re: [PATCH v3 4/6] Fix read / write data offsets in read / write loops
On Wed, Jun 21, 2017 at 09:29:14AM -0400, Steven Rostedt wrote: > On Wed, 14 Jun 2017 18:27:59 -0600 > Michael Sartain 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 > > --- > > 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 2/6] Fix unsigned return values being error checked as negative
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 <mikes...@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, , 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, , 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, ) < 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, ) < 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 = @@ -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, ) < 0) return -1; for (i = 0; i < count; i++) { - size = read8(handle); - if (size < 0) + if (read8(handle, ) < 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;
[PATCH v3 2/6] Fix unsigned return values being error checked as negative
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 --- 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, , 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, , 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, ) < 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, ) < 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 = @@ -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, ) < 0) return -1; for (i = 0; i < count; i++) { - size = read8(handle); - if (size < 0) + if (read8(handle, ) < 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
[PATCH v3 6/6] Fix cases where string literals were passed as string format args
Signed-off-by: Michael Sartain <mikes...@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(); - trace_dialog(parent, TRACE_GUI_OTHER, s.buffer); + trace_dialog(parent, TRACE_GUI_OTHER, "%s", s.buffer); } trace_seq_destroy(); 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, _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
[PATCH v3 1/6] Fix bad force_token escape sequence
Signed-off-by: Michael Sartain <mikes...@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
[PATCH v3 6/6] Fix cases where string literals were passed as string format args
Signed-off-by: Michael Sartain --- 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(); - trace_dialog(parent, TRACE_GUI_OTHER, s.buffer); + trace_dialog(parent, TRACE_GUI_OTHER, "%s", s.buffer); } trace_seq_destroy(); 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, _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
[PATCH v3 1/6] Fix bad force_token escape sequence
Signed-off-by: Michael Sartain --- 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
[PATCH v3 4/6] Fix read / write data offsets in read / write loops
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 <mikes...@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
[PATCH v3 4/6] Fix read / write data offsets in read / write loops
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 --- 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
[PATCH v3 3/6] Handle EINTR signal interrupts for read, write, open calls
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 <mikes...@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
[PATCH v3 3/6] Handle EINTR signal interrupts for read, write, open calls
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 --- 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
[PATCH v3 5/6] Fix function prototypes for __vwarning, __vpr_stat, and __vdie
They were declared: (const char *fmt, ...) but implemented as: (const char *fmt, va_list ap) Signed-off-by: Michael Sartain <mikes...@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 #include +#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 5/6] Fix function prototypes for __vwarning, __vpr_stat, and __vdie
They were declared: (const char *fmt, ...) but implemented as: (const char *fmt, va_list ap) Signed-off-by: Michael Sartain --- 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 #include +#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 0/6] trace-cmd: escape sequence, EINTR, error checking bug fixes
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 0/6] trace-cmd: escape sequence, EINTR, error checking bug fixes
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 v2 0/6] trace-cmd: escape sequence, EINTR, error checking bug fixes
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]. Please holler with any feedback. Thanks much. -Mike 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 >From d32d37c10ce0912851e1fb78d1e0c946bcd9 Mon Sep 17 00:00:00 2001 From: Michael Sartain <mikes...@fastmail.com> Date: Tue, 6 Jun 2017 11:08:13 -0600 Subject: [PATCH v2 1/6] Fix bad force_token escape sequence To: Steven Rostedt <rost...@goodmis.org> Cc: linux-kernel@vger.kernel.org Signed-off-by: Michael Sartain <mikes...@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 >From 370b3b40b5a1177a6f420013abd8196ef3f51034 Mon Sep 17 00:00:00 2001 From: Michael Sartain <mikes...@fastmail.com> Date: Tue, 6 Jun 2017 13:27:31 -0600 Subject: [PATCH v2 2/6] Fix unsigned return values being error checked as negative To: Steven Rostedt <rost...@goodmis.org> Cc: linux-kernel@vger.kernel.org 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 <mikes...@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, , 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, , 8)) return -1; - return __data2host8(pevent, data); + *size = __data2host8(pevent, data); + return 0;
[PATCH v2 0/6] trace-cmd: escape sequence, EINTR, error checking bug fixes
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]. Please holler with any feedback. Thanks much. -Mike 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 >From d32d37c10ce0912851e1fb78d1e0c946bcd9 Mon Sep 17 00:00:00 2001 From: Michael Sartain Date: Tue, 6 Jun 2017 11:08:13 -0600 Subject: [PATCH v2 1/6] Fix bad force_token escape sequence To: Steven Rostedt Cc: linux-kernel@vger.kernel.org Signed-off-by: Michael Sartain --- 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 >From 370b3b40b5a1177a6f420013abd8196ef3f51034 Mon Sep 17 00:00:00 2001 From: Michael Sartain Date: Tue, 6 Jun 2017 13:27:31 -0600 Subject: [PATCH v2 2/6] Fix unsigned return values being error checked as negative To: Steven Rostedt Cc: linux-kernel@vger.kernel.org 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 --- 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, , 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, , 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 *h
[PATCH] trace-cmd: Fix bad force_token escape sequence
Removes extra backslash in format string Signed-off-by: Michael Sartain <mikes...@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
[PATCH] trace-cmd: Fix bad force_token escape sequence
Removes extra backslash in format string Signed-off-by: Michael Sartain --- 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