Hi Tom,
On Tue, 13 Oct 2020 09:17:51 -0500
Tom Zanussi <zanu...@kernel.org> wrote:
> This updates v2 to replace some of the v2 code with improved code from
> Steve (tracing: Add synthetic event error logging) and (tracing:
> Handle synthetic event array field type checking correctly) and remove
> the synth_error_clear() function and change last_cmd_set() to use
> strncpy.
Thank you for updating, I tested the series and confirmed all issues
are fixed now :)
Tested-by: Masami Hiramatsu <mhira...@kernel.org>
for this series.
/ # cd /sys/kernel/debug/tracing/
/sys/kernel/debug/tracing # echo 'myevent char foo]' >> synthetic_events
sh: write error: Invalid argument
/sys/kernel/debug/tracing # cat error_log
[ 35.086780] synthetic_events: error: Illegal name
Command: myevent char foo]
^
/sys/kernel/debug/tracing # echo 'myevent char foo;[]' >> synthetic_events
sh: write error: Invalid argument
/sys/kernel/debug/tracing # cat error_log
[ 35.086780] synthetic_events: error: Illegal name
Command: myevent char foo]
^
[ 46.857674] synthetic_events: error: Illegal name
Command: myevent char foo;[]
^
/sys/kernel/debug/tracing # echo 'myevent char foo+[]' >> synthetic_events
sh: write error: Invalid argument
/sys/kernel/debug/tracing # cat error_log
[ 35.086780] synthetic_events: error: Illegal name
Command: myevent char foo]
^
[ 46.857674] synthetic_events: error: Illegal name
Command: myevent char foo;[]
^
[ 57.220147] synthetic_events: error: Illegal name
Command: myevent char foo+[]
^
/sys/kernel/debug/tracing # echo 'myevent char foo[]' >> synthetic_events
/sys/kernel/debug/tracing # cat synthetic_events
myevent char[] foo
/sys/kernel/debug/tracing # echo > synthetic_events
/sys/kernel/debug/tracing # echo 'myevent char[] foo' >> synthetic_events
/sys/kernel/debug/tracing # cat synthetic_events
myevent char[] foo
/sys/kernel/debug/tracing # cat events/synthetic/myevent/format
name: myevent
ID: 1219
format:
field:unsigned short common_type; offset:0; size:2;
signed:0;
field:unsigned char common_flags; offset:2; size:1;
signed:0;
field:unsigned char common_preempt_count; offset:3; size:1;
signed:0;
field:int common_pid; offset:4; size:4; signed:1;
field:__data_loc char[] foo; offset:8; size:8; signed:1;
print fmt: "foo=%.*s", __get_str(foo)
>
> Thanks,
>
> Tom
>
> v2 text:
>
> This updates v1 to fix a couple of additional things that Masami
> pointed out:
>
> - The error logging for the BAD_TYPE error was actually pointing to
> the name - it now points to the type as it should.
>
> - Added a new test case that verifies most of the synthetic event
> error messages and caret positions.
>
> - Added a new patch that correctly strips off trailing semicolons and
> everything else from array types, which wasn't happening before,
> even before the dynamic array patches.
>
> Original v1 text:
>
> These patches provide fixes for the problems observed by Masami in the
> new synthetic event dynamic string patchset.
>
> The first patch (tracing: Don't show dynamic string internals in
> synthetic event description) removes the __data_loc from the event
> description but leaves it in the format.
>
> The patch (tracing: Add synthetic event error logging) addresses the
> lack of error messages when parse errors occur.
>
> The remaining three patches address the other problems Masami noted
> which result from allowing illegal characters in synthetic event and
> field names when defining an event. The is_good_name() function is
> used to check that's not possible for the probe events, but should
> also be used for the synthetic events as well.
>
> (tracing: Move is_good_name() from trace_probe.h to trace.h) makes
> that function available to other trace subsystems by putting it in
> trace.h. (tracing: Check that the synthetic event and field names are
> legal) applies it to the synthetic events, and (selftests/ftrace:
> Change synthetic event name for inter-event-combined test) changes a
> testcase that now fails because it uses an illegal name.
>
> The following changes since commit 848183553e431e6e9c2ea2f72421a7a1bbc6532e:
>
> tracing: Fix synthetic print fmt check for use of __get_str() (2020-10-08
> 15:29:07 -0400)
>
> are available in the Git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/zanussi/linux-trace.git
> ftrace/dynstring-fixes-v3
>
> Tom Zanussi (7):
> tracing: Don't show dynamic string internals in synthetic event
> description
> tracing: Move is_good_name() from trace_probe.h to trace.h
> tracing: Check that the synthetic event and field names are legal
> tracing: Add synthetic event error logging
> selftests/ftrace: Change synthetic event name for inter-event-combined
> test
> tracing: Handle synthetic event array field type checking correctly
> selftests/ftrace: Add test case for synthetic event syntax errors
>
> kernel/trace/trace.h | 13 ++
> kernel/trace/trace_events_synth.c | 123 +++++++++++++++++-
> kernel/trace/trace_probe.h | 13 --
> .../trigger-inter-event-combined-hist.tc | 8 +-
> .../trigger-synthetic_event_syntax_errors.tc | 19 +++
> 5 files changed, 153 insertions(+), 23 deletions(-)
> create mode 100644
> tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-synthetic_event_syntax_errors.tc
>
> --
> 2.17.1
>
--
Masami Hiramatsu <mhira...@kernel.org>
