Re: [PATCH v2 22/24] selftests-dyndbg: add a dynamic_debug run_tests target

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Test failure conditions incorrectly exit with a success status (0), silently hiding test failures from the kselftest framework.
- [Medium] The script incorrectly assumes `.config` is available at runtime, which will cause false test failures when selftests are installed and run on a target device.
--

commit 5991b9725cc8742537ed8b6a73204e4d44da0fc2
Author: Jim Cromie <jim.cromie@gmail.com>

selftests-dyndbg: add a dynamic_debug run_tests target

This commit adds a dynamic-debug selftest script that modifies pr_debug()
flags and checks their effects. It also integrates dynamic_debug into the
kselftest framework's TARGETS list.

> diff --git a/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh b/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
> new file mode 100755
> index 0000000000000..01c035fe8c9a2
> --- /dev/null
> +++ b/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
> @@ -0,0 +1,257 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0-only

[ ... ]

> +[ -f "$KCONFIG_CONFIG" ] || KCONFIG_CONFIG=".config"
> +if [ -f "$KCONFIG_CONFIG" ]; then

[Severity: Medium]
Will this work reliably when selftests are installed and run on a target
device?  When tests are run via `make install` on a target, the source tree's
`.config` won't be available.

If the `.config` is missing, the script defaults to `LACK_DD_BUILTIN=0`, which
may cause false test failures if the target kernel doesn't actually have the
config enabled. Could runtime indicators be checked instead of build-time files?

> +    echo "# consulting KCONFIG_CONFIG: $KCONFIG_CONFIG"
> +    grep -q "CONFIG_DYNAMIC_DEBUG=y" $KCONFIG_CONFIG ; LACK_DD_BUILTIN=$?
> +    grep -q "CONFIG_TEST_DYNAMIC_DEBUG=m" $KCONFIG_CONFIG ; LACK_TMOD=$?

[ ... ]

> +function handle_exit_code() {
> +    local exp_exit_code=0
> +    [ $# == 4 ] && exp_exit_code=$4
> +    if [ $3 -ne $exp_exit_code ]; then
> +        echo -e "${RED}: $BASH_SOURCE:$1 $2() expected to exit with code $exp_exit_code"
> +	[ $3 == 1 ] && echo "Error: '$error_msg'"
> +        exit $exp_exit_code

[Severity: Medium]
Does this mask test failures?  If the actual exit code (`$3`) does not match
`exp_exit_code` (which defaults to 0), the script calls `exit $exp_exit_code`.

Since `exp_exit_code` is typically 0 when expecting a command to succeed, the
script will exit with 0 upon encountering an error, silently hiding the failure
from the kselftest framework.

> +    fi
> +}

[ ... ]

> +function check_match_ct {
> +    pattern="\s$1\s"
> +    exp_cnt=0
> +
> +    [ "$3" == "-r" ] && pattern="$1"
> +    let cnt=$(ddgrep "$pattern" | wc -l)
> +    if [ $V -eq 1 ] || [ "$3" == "-v" ] || [ "$4" == "-v" ]; then
> +        echo -ne "${BLUE}" && ddgrep $pattern && echo -ne "${NC}"
> +    fi
> +    [ $# -gt 1 ] && exp_cnt=$2
> +    if [ $cnt -ne $exp_cnt ]; then
> +        echo -e "${RED}: $BASH_SOURCE:$BASH_LINENO check failed expected $exp_cnt on $1, got $cnt"
> +        exit

[Severity: Medium]
Does this also mask test failures?  Calling `exit` without arguments returns
the status of the last executed command.

Since the `echo` command immediately preceding it will succeed with a 0 exit
code, the script exits with 0 and the kselftest framework will incorrectly mark
the failed test as passed. This same pattern appears to be used in several
other functions in the script.

> +    else
> +        echo ": $cnt matches on $1"
> +    fi
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260523-dd-maint-2-v2-0-b937312aa083@gmail.com?part=22