Re: [PATCH v2 5/7] perf test: Enhance annotate test coverage and isolate config

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Uninitialized variable and incompatible data format causes `test_disassembler` to unconditionally fail.
--

commit 5f0bdfa4980785631dd9a8024ff9f43baa234caa
Author: Ian Rogers <irogers@google.com>

perf test: Enhance annotate test coverage and isolate config

This commit updates the annotate.sh test to support different disassembler
backends via the new --disassembler command-line option. It also isolates the
test script from host environment pollution by exporting PERF_CONFIG=/dev/null.

> diff --git a/tools/perf/tests/shell/annotate.sh b/tools/perf/tests/shell/annotate.sh
> index 689de58e92384..2c06d2670cb18 100755
> --- a/tools/perf/tests/shell/annotate.sh
> +++ b/tools/perf/tests/shell/annotate.sh
[ ... ]
> +test_disassembler() {
> +  disassembler=$1
> +  feature=$2
[ ... ]
> +  perf annotate --no-demangle -i "${perfdata}" --stdio --percent-limit 10 --disassembler "${disassembler}" 2> /dev/null > "${perfout}" || ret=$?
> +
> +  if [ "x${ret}" != "x0" ]
> +  then
> +    echo "annotate with ${disassembler} [Failed: perf annotate error]"
> +    err=1
> +    return 0
> +  fi

[Severity: Medium]
Does this evaluation unconditionally fail the test when perf annotate succeeds?

If the perf annotate command is successful, the short-circuit `|| ret=$?` means
ret is skipped and never assigned. Because ret is not initialized as a local
variable at the start of test_disassembler(), it remains empty. The subsequent
check then evaluates to `[ "x" != "x0" ]`, which is always true and incorrectly
flags a success as a failure.

Additionally, will reading `${perfdata}` as a regular file here cause an issue
due to the test execution order?

>  test_basic Basic
>  test_basic Pipe
>  
> +if [ "${err}" -eq 0 ]
> +then
> +  test_disassembler "objdump" ""

The test_basic sequence runs `test_basic Pipe` immediately before executing
test_disassembler(). This overwrites `${perfdata}` with a pipe-format stream,
so attempting to read it using `-i "${perfdata}"` inside test_disassembler()
may cause perf annotate execution failures.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609070732.545416-1-irogers@google.com?part=5