Re: [PATCH v3 3/3] dt-bindings: add self-test fixtures for style checker

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The test runner script `run.sh` uses bash-specific process substitution (`<()`) but specifies `#!/bin/sh` as the interpreter.
- [Medium] The test runner script `run.sh` ignores the exit status of the `dt-check-style` tool, contradicting its own documented requirements.
--

commit 395b561606d87e6d15dae9a82c1d4e309bfe406d
Author: Daniel Golle <daniel@makrotopia.org>

dt-bindings: add self-test fixtures for style checker

This commit provides good and bad DTS and YAML fixtures along with a test
runner to verify the behavior of dt-check-style. It also wires this up to a
new top-level make target, dt_style_selftest.

> diff --git a/scripts/dtc/dt-style-selftest/run.sh b/scripts/dtc/dt-style-selftest/run.sh
> new file mode 100755
> index 0000000000000..821d635ddc151
> --- /dev/null
> +++ b/scripts/dtc/dt-style-selftest/run.sh
> @@ -0,0 +1,67 @@
> +#!/bin/sh

Since this script specifies /bin/sh, which on systems like Debian and Ubuntu
points to dash, will this cause a syntax error when evaluating the
bash-specific process substitution used later in the script?

> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# Run dt-check-style against fixtures under good/ and bad/.
> +# good/ files must produce no output and exit 0 in both modes.
> +# bad/ files must produce the expected output (in expected/<name>.txt)
> +# and exit 1.

[ ... ]

> +for f in "$here"/good/*; do
> +    [ -e "$f" ] || continue
> +    for mode in relaxed strict; do
> +        out=$(run "$f" "$mode") || true

Does ignoring the exit status here and below conflict with the requirements
documented at the top of the script?

By appending || true, the script seems to rely only on the output string
matching. This would allow tests to silently pass even if dt-check-style
exits with an incorrect code or crashes entirely.

[ ... ]

> +for f in "$here"/bad/*; do

[ ... ]

> +    out=$(run "$f" "$mode") || true
> +    # Strip the directory prefix so expected files are portable.
> +    out=$(printf '%s\n' "$out" | sed "s|$here/bad/|bad/|g")
> +    if [ "$out" != "$body" ]; then
> +        echo "FAIL bad/$mode: $name:"
> +        diff -u <(printf '%s\n' "$body") <(printf '%s\n' "$out") | \

Can this process substitution syntax <() cause a fatal syntax error on
systems where /bin/sh is a strict POSIX shell?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1778454442.git.daniel@makrotopia.org?part=3