Re: [PATCH 4/4] perf tests c2c: Add function view stdio coverage

[email protected]
Newsgroups org.kernel.vger.linux-perf-users
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly added `test_c2c_function_report` test fails spuriously because it incorrectly assumes `perfdata` is valid when `err` is 0. If the workload in `test_c2c_record_report` fails, it returns early but leaves `err=0`. This causes `test_c2c_function_report` to process an invalid or missing `perfdata` file, failing its diagnostic checks and setting `err=1` (test failure).
--

--- Patch [4]: [PATCH 4/4] perf tests c2c: Add function view stdio coverage ---
commit 32c977aa30c2c6186f39077056298ef3b53307dd
Author: Jiebin Sun <[email protected]>

perf tests c2c: Add function view stdio coverage

Exercise the function view without driving a terminal now that it has
a stdio path. Keep the existing datasym record/report coverage and make a
separate recording of the contended locks used by the futex hash benchmark.

> diff --git a/tools/perf/tests/shell/c2c.sh b/tools/perf/tests/shell/c2c.sh
> index f5f223cbf9cc7..cf76359d19ccd 100755
> --- a/tools/perf/tests/shell/c2c.sh
> +++ b/tools/perf/tests/shell/c2c.sh
[ ... ]
> @@ -58,6 +63,114 @@ test_c2c_record_report() {
>  	echo "c2c record and report test [Success]"
>  }
>  
> +test_c2c_function_report() {
> +	echo "c2c function stdio report test"
> +
> +	if perf c2c report -i "${perfdata}" --function -c pid > "${perfout}" 2>&1 ; then
> +		echo "c2c function stdio report test [Failed: report accepted missing iaddr]"
> +		err=1
> +		return
> +	fi
[ ... ]
>  test_c2c_record_report
> +if [ "${err}" -eq 0 ]; then
> +	test_c2c_function_report
> +fi
>  cleanup
>  exit $err

[Severity: Medium]
Will this lead to a spurious test failure if the record workload fails?

In test_c2c_record_report(), if the 'perf test -w datasym 1' workload fails,
it returns early without modifying err, leaving err=0:

	if ! perf c2c record -o "${perfdata}" -- perf test -w datasym 1 > /dev/null 2>&1 ; then
		echo "c2c record and report test [Skipped: perf c2c record failed during workload]"
		return
	fi

Because err remains 0, test_c2c_function_report() will conditionally execute
and attempt to use the missing or invalid perfdata file. This will cause
test_c2c_function_report() to fail its diagnostic checks and set err=1.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.