Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] selftests/ftrace: negated pipelines in filter-syntax-check.tc bypass set -e --
commit 98a96a63f4d758b8db11a7d2138febf6cb5cf6e9 Author: Masami Hiramatsu (Google) <[email protected]> selftests/ftrace: Add event filter syntax check test case This patch adds filter-syntax-check.tc to test valid event filter expressions, error_log caret reporting for malformed filter expressions, newline truncation on multi-line filter input, and error returns on invalid subsystem filters. > diff --git > a/tools/testing/selftests/ftrace/test.d/filter/filter-syntax-check.tc > b/tools/testing/selftests/ftrace/test.d/filter/filter-syntax-check.tc > new file mode 100644 > index 0000000000000..68c8accc5ffd0 > --- /dev/null > +++ b/tools/testing/selftests/ftrace/test.d/filter/filter-syntax-check.tc > @@ -0,0 +1,52 @@ [ ... ] > +# Multi-line input must be truncated at the first newline > +echo > error_log > +! ( printf "common_pid == CPUS{0\n\"} && common_pid == 0 && common_pid == > CPUS{0\n\"}" | \ > + dd of=events/sched/sched_switch/filter bs=4096 ) 2> /dev/null > +grep -q "Missing '}'" error_log > +! grep -q "Missing matching quote" error_log > + > +# Subsystem filter must fail on invalid filter expressions > +echo 'common_pid >= 0' > events/sched/filter > +! echo 'no_such_field == 1' > events/sched/filter > +! echo 'common_pid == 1 &&' > events/sched/filter > +echo 0 > events/sched/filter > + > +exit 0 [Severity: Medium] Will these negated pipelines actually fail the test if any of the inner commands unexpectedly succeed? In tools/testing/selftests/ftrace/ftracetest, __run_test() sources each test script in a /bin/sh subshell with set -e enabled: __run_test() { # testfile # setup PID and PPID, $$ is not updated. (cd $TRACING_DIR; read PID _ < /proc/self/stat; set -e; set -x; checkreq $1; initialize_system; . $1) [ $? -ne 0 ] && kill -s $SIG_FAIL $SIG_PID } In POSIX sh, set -e is ignored for any pipeline beginning with the ! reserved word. If the dd, grep, or echo commands in filter-syntax-check.tc unexpectedly return 0, the negated pipeline evaluates to status 1 without causing the shell to exit, and execution continues through: echo 0 > events/sched/filter exit 0 allowing the test to pass even when a negative check fails. Should these use explicit failure handling (such as || exit_fail) or wrap the negation in a subshell so set -e catches an unexpected success? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/179291742944.492959.11659339795290028998.stgit@devnote2?part=9
