From: sashiko-bot@kernel.org
To: "Krzysztof Kozlowski" <krzysztof.kozlowski@oss.qualcomm.com>
Cc: robh@kernel.org, conor+dt@kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH v5 2/7] dtc: dt-check-style: Allow space-aligning indentation in DTS
Date: Thu, 09 Jul 2026 18:01:18 +0000 [thread overview]
Message-ID: <20260709180118.6935B1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260709-dts-style-checker-v5-2-fcc147cb697d@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The check for excessive space characters (> 7) is not applied to continuation lines, allowing invalid mixed indentation to silently pass.
- [Low] Dead code: Unreachable `LineType.PREPROCESSOR` check inside continuation loop.
--
--- Patch [2]: [PATCH v5 2/7] dtc: dt-check-style: Allow space-aligning indentation in DTS ---
commit a9c24c0264c4f952c0bab2866c36e42454e6fbde
Author: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
dtc: dt-check-style: Allow space-aligning indentation in DTS
This commit modifies the dt-check-style script to allow spaces after tabs
in DTS indentation for aligning continued lines. It limits spaces to a
maximum of 7 and explicitly disallows a space followed by a tab.
> --- a/scripts/dtc/dt-check-style
> +++ b/scripts/dtc/dt-check-style
> @@ -359,14 +359,24 @@ def check_tab_in_yaml_example(ctx):
>
>
> def check_mixed_indent_chars(ctx):
[ ... ]
> + if dl.indent_str.count(' ') > 7:
> + yield (dl.lineno, 'too many space characters in indent (more than 7)')
> + for cont in dl.continuations:
> + if not cont.indent_str:
> + continue
> + if cont.linetype == LineType.PREPROCESSOR:
> + continue
[Severity: Low]
Are preprocessor lines possible in this continuation list? The logic that
builds the list seems to guarantee that continuations only ever contain
LineType.CONTINUATION elements, as preprocessor directives reset the
continuation state and are added to the main line list instead.
> + if re.search(r' \t', cont.indent_str):
> + yield (cont.lineno, 'mixed tabs and spaces in indent')
[Severity: Medium]
Should this loop over continuations also enforce the check for excessive
space characters? While the primary line is checked for more than 7 spaces
above, continuation lines omit this validation, which could allow invalid
mixed indentation to pass silently.
[ ... ]
> --- /dev/null
> +++ b/scripts/dtc/dt-style-selftest/bad/dts-mixed-indent.dts
> @@ -0,0 +1,21 @@
[ ... ]
> + clocks = <1>,
> + <2>,
> + <3>,
> + <4>;
[Severity: Medium]
Does the continuation line with value <4> contain 9 space characters?
Because the excessive space check is omitted for continuations in the
dt-check-style script, this 9-space indentation goes undetected.
> + resets = <5>;
> + };
> +};
> diff --git a/scripts/dtc/dt-style-selftest/expected/dts-mixed-indent.dts.txt b/scripts/dtc/dt-style-selftest/expected/dts-mixed-indent.dts.txt
> new file mode 100644
> index 0000000000000..93146cfb51c78
> --- /dev/null
> +++ b/scripts/dtc/dt-style-selftest/expected/dts-mixed-indent.dts.txt
> @@ -0,0 +1,9 @@
[ ... ]
> +bad/dts-mixed-indent.dts:13: [mixed-indent-chars] mixed tabs and spaces in indent
> +bad/dts-mixed-indent.dts:16: [mixed-indent-chars] mixed tabs and spaces in indent
> +bad/dts-mixed-indent.dts:19: [indent-consistent] indent mismatch (expected depth 2 * '\t')
[Severity: Medium]
Should this expected output file also be updated once the excessive space
check is applied to the continuations loop? It currently fails to assert a
mixed-indent-chars warning for the 9-space continuation in the clocks
property above.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-dts-style-checker-v5-0-fcc147cb697d@oss.qualcomm.com?part=2
next prev parent reply other threads:[~2026-07-09 18:01 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-09 17:41 [PATCH v5 0/7] dtc: dt-check-style: Improvement after using on DTS Krzysztof Kozlowski
2026-07-09 17:41 ` [PATCH v5 1/7] dtc: dt-check-style: Narrow disallowing of tab in DTS only to YAML Krzysztof Kozlowski
2026-07-09 17:41 ` [PATCH v5 2/7] dtc: dt-check-style: Allow space-aligning indentation in DTS Krzysztof Kozlowski
2026-07-09 18:01 ` sashiko-bot [this message]
2026-07-09 17:41 ` [PATCH v5 3/7] dtc: dt-check-style: Rework handling YAML/DTS in rules Krzysztof Kozlowski
2026-07-09 17:41 ` [PATCH v5 4/7] dtc: dt-check-style: Expect first device_type Krzysztof Kozlowski
2026-07-09 17:41 ` [PATCH v5 5/7] dtc: dt-check-style: Handle properly DTC-style includes Krzysztof Kozlowski
2026-07-09 17:41 ` [PATCH v5 6/7] dtc: dt-check-style: Print proper line number of indentation detection place Krzysztof Kozlowski
2026-07-09 17:41 ` [PATCH v5 7/7] dtc: dt-check-style: Add more DTS test cases Krzysztof Kozlowski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260709180118.6935B1F000E9@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=krzysztof.kozlowski@oss.qualcomm.com \
--cc=robh@kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox