Re: [PATCH v2 9/9] selftests/ftrace: Add event filter syntax check test case

[sashiko-bot@kernel.org]

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) <mhiramat@kernel.org>

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