Re: [PATCH 3/9] dtc: dt-check-style: Handle root node in overlays

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The patch introduces a regression for valid root nodes (e.g., labeled or with extra spacing) and fails to change behavior for overlay root nodes, rendering the intended fix ineffective and actively harmful.
- [Medium] The patch claims to fix "property order" false positives but exclusively modifies "child name order" logic, leaving property order checking unchanged.
--

--- Patch [3]: [PATCH 3/9] dtc: dt-check-style: Handle root node in overlays ---
commit 3ffd10d90b05e9e479a121a83377b237c94b4b24
Author: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

dtc: dt-check-style: Handle root node in overlays

This patch modifies the device tree style checker to detect root nodes in
overlays by matching specific node open strings. It stores this state in an
is_root attribute and uses it when checking child name order.

> This fixes false positives for property order for root nodes in overlays.

[Severity: Medium]
The commit message states this patch fixes "property order" false
positives, but the code exclusively modifies the child name order logic in
check_child_name_order(). Is check_property_order() supposed to use the new
is_root flag as well?

> diff --git a/scripts/dtc/dt-check-style b/scripts/dtc/dt-check-style
> --- a/scripts/dtc/dt-check-style
> +++ b/scripts/dtc/dt-check-style

[ ... ]

> @@ -228,7 +229,10 @@ def classify_lines(text):
>              continue
>  
>          if code.endswith('{'):
> -            dl = DtsLine(i, raw, LineType.NODE_OPEN, depth, indent_str, code)
> +            is_root = False
> +            if code == '&{/} {' or code == '/ {':

[Severity: Medium]
Will this exact string match handle valid alternative spacings or labels?
For example, if the code contains a label like "label: / {" or extra
whitespace like "/  {", it seems this would fail to set is_root to True.

> +                is_root = True
> +            dl = DtsLine(i, raw, LineType.NODE_OPEN, depth, indent_str, code, is_root=is_root)
>              parse_node_header(dl)
>              out.append(dl)
>              depth += 1

[ ... ]

> @@ -550,7 +554,9 @@ def check_child_name_order(ctx):
>          for c in children:
>              if c.node_addr is not None:
>                  continue
> -            if c.node_name in (None, '/'):
> +            if c.node_name is None:
> +                continue
> +            if c.is_root:

[Severity: Medium]
Since this replaces the previous check for c.node_name in (None, '/'), could
it cause valid root nodes that were missed by the exact string match in
classify_lines() to incorrectly bypass this skip logic and trigger false
positives?

>                  continue
>              if c.ref_name is not None:
>                  continue

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260726-n-dts-style-checker-continued-v1-0-b05b8f897670@oss.qualcomm.com?part=3