| Message ID | 20211119112420.4826-1-vincent.whitchurch@axis.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | trace-cmd report: Avoid crash on unknown event | expand |
On Fri, 19 Nov 2021 12:24:20 +0100 Vincent Whitchurch <vincent.whitchurch@axis.com> wrote: > Do not segfault if the event cannot be found for some reason and > tep_find_event_by_record() returns NULL. > > No extra warning is added since there are others ("UNKNOWN EVENT") which > already make it clear that something is wrong: > > kworker/u8:0-7 [003] 1.245773: sched_stat_runtime: comm=kworker/u8:... > [UNKNOWN EVENT][UNKNOWN EVENT][UNKNOWN EVENT] > kworker/u8:0-7 [003] 1.245776: sched_switch: kworker/u8:0:7 [120] W... > I wonder if the following would be a better and more robust approach: diff --git a/tracecmd/trace-read.c b/tracecmd/trace-read.c index f7ffb89e..4b27740a 100644 --- a/tracecmd/trace-read.c +++ b/tracecmd/trace-read.c @@ -142,12 +142,15 @@ static struct trace_hash wakeup_hash; static void print_event_name(struct trace_seq *s, struct tep_event *event) { static const char *spaces = " "; /* 20 spaces */ + const char *name; int len; - trace_seq_printf(s, " %s: ", event->name); + name = event ? event->name : "(NULL)"; + + trace_seq_printf(s, " %s: ", name); /* Space out the event names evenly. */ - len = strlen(event->name); + len = strlen(name); if (len < 20) trace_seq_printf(s, "%.*s", 20 - len, spaces); } -- Steve
On Fri, Nov 19, 2021 at 03:44:34PM +0100, Steven Rostedt wrote: > On Fri, 19 Nov 2021 12:24:20 +0100 > Vincent Whitchurch <vincent.whitchurch@axis.com> wrote: > > > Do not segfault if the event cannot be found for some reason and > > tep_find_event_by_record() returns NULL. > > > > No extra warning is added since there are others ("UNKNOWN EVENT") which > > already make it clear that something is wrong: > > > > kworker/u8:0-7 [003] 1.245773: sched_stat_runtime: comm=kworker/u8:... > > [UNKNOWN EVENT][UNKNOWN EVENT][UNKNOWN EVENT] > > kworker/u8:0-7 [003] 1.245776: sched_switch: kworker/u8:0:7 [120] W... > > > > I wonder if the following would be a better and more robust approach: > > diff --git a/tracecmd/trace-read.c b/tracecmd/trace-read.c > index f7ffb89e..4b27740a 100644 > --- a/tracecmd/trace-read.c > +++ b/tracecmd/trace-read.c > @@ -142,12 +142,15 @@ static struct trace_hash wakeup_hash; > static void print_event_name(struct trace_seq *s, struct tep_event *event) > { > static const char *spaces = " "; /* 20 spaces */ > + const char *name; > int len; > > - trace_seq_printf(s, " %s: ", event->name); > + name = event ? event->name : "(NULL)"; > + > + trace_seq_printf(s, " %s: ", name); > > /* Space out the event names evenly. */ > - len = strlen(event->name); > + len = strlen(name); > if (len < 20) > trace_seq_printf(s, "%.*s", 20 - len, spaces); > } Looks good to me.
On Tue, 23 Nov 2021 09:55:14 +0100
Vincent Whitchurch <vincent.whitchurch@axis.com> wrote:
> Looks good to me.
Want to submit a v2 and add:
Suggested-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
?
-- Steve
diff --git a/tracecmd/trace-read.c b/tracecmd/trace-read.c index 6f43c1d..145c823 100644 --- a/tracecmd/trace-read.c +++ b/tracecmd/trace-read.c @@ -890,7 +890,8 @@ void trace_show_data(struct tracecmd_input *handle, struct tep_record *record) trace_seq_printf(&s, " %-8s", buf); } - print_event_name(&s, event); + if (event) + print_event_name(&s, event); tep_print_event(pevent, &s, record, "%s", format_type); if (s.len && *(s.buffer + s.len - 1) == '\n')
Do not segfault if the event cannot be found for some reason and tep_find_event_by_record() returns NULL. No extra warning is added since there are others ("UNKNOWN EVENT") which already make it clear that something is wrong: kworker/u8:0-7 [003] 1.245773: sched_stat_runtime: comm=kworker/u8:... [UNKNOWN EVENT][UNKNOWN EVENT][UNKNOWN EVENT] kworker/u8:0-7 [003] 1.245776: sched_switch: kworker/u8:0:7 [120] W... Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com> --- tracecmd/trace-read.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)