Re: [PATCH v5 1/4] dt-bindings: add DTS style checker

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] `check_mixed_indent_chars` incorrectly flags property continuations and block comments as mixed indent violations, causing widespread false positives.
- [Medium] Multi-closure lines (`}; };`) break tree traversal in structural checks.
- [Medium] Regex ordering in `collect_labels_and_refs` corrupts label detection when strings contain comment sequences.
--

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

dt-bindings: add DTS style checker

This patch adds a Python tool that checks DTS coding style on examples in
YAML binding files and on DTS source files. It introduces rules defined in
a declarative registry for relaxed and strict mode checking.

> diff --git a/scripts/dtc/dt-check-style b/scripts/dtc/dt-check-style
> new file mode 100755
> index 0000000000000..2d5723d41ea39
> --- /dev/null
> +++ b/scripts/dtc/dt-check-style
[ ... ]
> +        if re_only_closures.match(code):
> +            closures = code.count('}')
> +            depth = max(depth - closures, 0)
> +            dl = DtsLine(i, raw, LineType.NODE_CLOSE, indent_str, code)
> +            dl.depth = depth
> +            dl.closures = closures
> +            out.append(dl)
> +            prev_complete = True
> +            continue
[ ... ]
> +def check_property_order(ctx):
> +    """Properties within a node body must appear in canonical order:
[ ... ]
> +    for i, dl in enumerate(lines):
> +        if dl.linetype != LineType.NODE_OPEN:
> +            continue
> +        body_depth = dl.depth + 1
> +        props = []
> +        for j in range(i + 1, len(lines)):
> +            d = lines[j]
> +            if d.linetype == LineType.NODE_CLOSE and \
> +                    d.depth == body_depth - 1:
> +                break

[Severity: Medium]
When a line contains multiple closures like "}; };", classify_lines()
decrements the depth by the number of closures.

Will this cause the depth to drop by 2 or more, skipping the exact
termination condition of d.depth == body_depth - 1?

It looks like tree traversal in structural checks like check_property_order()
or _walk_bodies() might miss the end of the node body and improperly mix
properties from different siblings.

[ ... ]
> +def collect_labels_and_refs(text):
> +    """Return (defined_labels, referenced_labels) found anywhere outside
> +    /* */ comments and string literals. Labels named fake_intc* (injected
> +    by dt-extract-example) are skipped."""
> +    # Strip block comments first so labels inside them don't count
> +    stripped = re.sub(r'/\*.*?\*/', '', text, flags=re.DOTALL)
> +    # Strip line comments
> +    stripped = re.sub(r'//[^\n]*', '', stripped)
> +    # Strip string literals so words inside quotes (e.g. "Error: foo")
> +    # are not picked up as label definitions or &-references.
> +    stripped = re.sub(r'"(?:[^"\\]|\\.)*"', '""', stripped)

[Severity: Medium]
Since comments are stripped before string literals, does this break if a
string literal contains sequences like // or /*?

If a string contains a URL like "http://..." or a /* sequence without a
closing */, the comment regex might activate inside the string and consume
valid source code and labels up to the next */ somewhere else in the file,
causing false positives for the unused-labels check.

[ ... ]
> +def check_mixed_indent_chars(ctx):
> +    """Indent must be all-spaces or all-tabs, never mixed on one line."""
> +    for dl in ctx.lines:
> +        if not dl.indent_str:
> +            continue
> +        if dl.linetype == LineType.PREPROCESSOR:
> +            continue
> +        if ' ' in dl.indent_str and '\t' in dl.indent_str:
> +            yield (dl.lineno, 'mixed tabs and spaces in indent')

[Severity: Medium]
Does this code flag legitimate property continuations or block comments as
mixed indent violations?

Multi-line property continuations in DTS typically use tabs for base
indentation and spaces for visual alignment under a specific character,
which is an alignment pattern expected by check_continuation_alignment().

If check_mixed_indent_chars() flags these, it might cause widespread false
positives in the default relaxed mode.

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