Re: [PATCH v4 1/3] dt-bindings: add DTS style checker

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [Medium] Code following a block comment on the same line is completely ignored, breaking parsing state and nesting depth.
- [Medium] Multi-line C preprocessor directives (macros) using `\` continuations are mishandled, corrupting the parsing state and triggering false-positive style warnings.
- [Medium] The unused-labels checker regex matches inside string literals and misses labels on nodes starting with digits.
- [Medium] Syntactic checks operating on `dl.raw` incorrectly process the contents of unclosed block comments on `PROPERTY` lines, causing false positives.
- [Medium] The regex for node headers rejects nodes with malformed unit addresses, allowing them to bypass the unit-address format check.
- [Low] Multiple closing braces on the same line break depth tracking and cause cascading false positive warnings.
--

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

dt-bindings: add DTS style checker

Add a Python tool that checks DTS coding style on examples in YAML binding
files and on .dts/.dtsi/.dtso source files. Rules are kept in a declarative
registry, tagged as 'relaxed' or 'strict'.

> diff --git a/scripts/dtc/dt-check-style b/scripts/dtc/dt-check-style
> new file mode 100755
> index 0000000000000..93cb1d7d2985c
> --- /dev/null
> +++ b/scripts/dtc/dt-check-style

[ ... ]

> +re_node_header = re.compile(
> +    r'^(?:([a-zA-Z_][a-zA-Z0-9_]*):\s*)?'
> +    r'([a-zA-Z0-9][a-zA-Z0-9,._+-]*|/)'
> +    r'(?:@([0-9a-fA-F,]+))?'
> +    r'\s*\{$')

[Severity: Medium]
Will this regex reject nodes with malformed unit addresses, allowing them to
bypass the unit-address format check?

If the unit address contains characters outside this set, such as the x in
memory@0x1000, the entire regex match fails and dl.node_addr remains None.

Because check_unit_address_format() skips validation if dl.node_addr is None,
does this allow malformed unit addresses containing 0x to bypass the very
check designed to flag them?

[ ... ]

> +        if in_block_comment:
> +            ltype = (LineType.COMMENT_END if '*/' in stripped
> +                     else LineType.COMMENT_BODY)
> +            if ltype == LineType.COMMENT_END:
> +                in_block_comment = False
> +            dl = DtsLine(i, raw, ltype, indent_str, stripped)
> +            dl.depth = depth
> +            out.append(dl)
> +            continue
> +
> +        if stripped.startswith('/*'):
> +            if '*/' in stripped:
> +                ltype = LineType.COMMENT
> +            else:
> +                in_block_comment = True
> +                ltype = LineType.COMMENT_START
> +            dl = DtsLine(i, raw, ltype, indent_str, stripped)
> +            dl.depth = depth
> +            out.append(dl)
> +            continue

[Severity: Medium]
Does this code skip parsing for any valid DTS code following a block comment
on the exact same line?

If a line starts with /* and contains */, or if an open block comment ends on
the line, the script sets the line type and immediately executes continue.

Could this lead to desynchronized nesting depth tracking by ignoring
constructs like /* ... */ }; or /* ... */ foo = <1>;, incorrectly classifying
subsequent lines and triggering cascading false positive violations?

> +
> +        if stripped.startswith('//'):
> +            dl = DtsLine(i, raw, LineType.COMMENT, indent_str, stripped)
> +            dl.depth = depth
> +            out.append(dl)
> +            continue
> +
> +        if stripped.startswith('#') and is_preprocessor(stripped):
> +            dl = DtsLine(i, raw, LineType.PREPROCESSOR,
> +                         indent_str, stripped)
> +            dl.depth = depth
> +            out.append(dl)
> +            prev_complete = True
> +            continue

[Severity: Medium]
Does this logic mishandle multi-line C preprocessor directives using
backslash continuations?

If the directive has trailing backslashes to continue macros to subsequent
lines, the continuation lines do not start with #.

Are these improperly parsed as standard DTS properties or continuations,
inappropriately applying DTS constraints to internal C code and triggering
false positives?

[ ... ]

> +        if code == '};' or code == '}':
> +            depth = max(depth - 1, 0)
> +            dl = DtsLine(i, raw, LineType.NODE_CLOSE, indent_str, code)
> +            dl.depth = depth
> +            out.append(dl)
> +            prev_complete = True
> +            continue

[Severity: Low]
Will this fail to decrement nesting depth if a single line contains multiple
closures?

If code contains }; };, this exact-match condition fails.

Does the line then improperly fall through and get classified as
LineType.PROPERTY, corrupting depth tracking for subsequent lines and
incorrectly flagging them for indentation violations?

[ ... ]

> +def collect_labels_and_refs(text):
> +    """Return (defined_labels, referenced_labels) found anywhere outside
> +    /* */ comments. 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)
> +    defined = set()
> +    referenced = set()
> +    for m in re.finditer(r'(?:^|[\s{])([a-zA-Z_][a-zA-Z0-9_]*):\s*[a-zA-Z/&]',
> +                         stripped):

[Severity: Medium]
Does this regex match inside string literals and miss labels on nodes
starting with digits?

Since string literals are not stripped before searching for labels, words
followed by colons inside strings like prop = "Error: failed"; could be added
to the defined set and flagged as unused.

Additionally, because the regex requires the character following the colon to
be a letter, /, or &, does it fail to capture labels attached to nodes
starting with a digit like label: 1wire@10 {?

[ ... ]

> +def _strip_strings_and_comments(text):
> +    """Remove string literals and /* */ + // comments from a single
> +    line, replacing them with empty strings. Used so syntactic checks
> +    (whitespace, hex case, etc.) don't false-positive on contents of
> +    quoted strings or comments."""
> +    text = re.sub(r'"(?:[^"\\]|\\.)*"', '""', text)
> +    text = re.sub(r'/\*.*?\*/', '', text)
> +    text = re.sub(r'//.*$', '', text)
> +    return text

[Severity: Medium]
Will syntactic checks incorrectly process the contents of unclosed block
comments on PROPERTY lines?

Because the regex only removes block comments that both open and close on the
same line, if a line contains an unclosed block comment like
prop = <1>; /* ... 0xABC, the regex fails to match.

Does the comment text remain intact, causing checks like check_hex_case() to
yield false positive errors for uppercase hex values inside the comment?

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