Re: [PATCH v3 03/11] dtc: dt-check-style: Handle root node in overlays

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch fails to address the issue claimed in the commit message, introduces dead logic for overlay root nodes, and causes a regression for standard unspaced root nodes.
--

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

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

This patch aims to handle root nodes in overlays correctly by detecting
them and storing an is_root attribute in DtsLine, intending to fix false
positives for property order.

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

[Severity: Medium]
Does this patch actually update check_property_order()? 

The diff modifies check_child_name_order() instead, leaving the
check_property_order() function completely unchanged.

> diff --git a/scripts/dtc/dt-check-style b/scripts/dtc/dt-check-style
> index 96deffc0d8a7c..d9f76d6d9179e 100755
> --- 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 re.search(r'^/\s+\{$', code):
> +                is_root = True
> +            dl = DtsLine(i, raw, LineType.NODE_OPEN, depth, indent_str, code, is_root=is_root)

[Severity: Medium]
Will this regex handle standard unspaced root nodes like "/{"?

The regex pattern ^/\s+\{$ strictly requires whitespace. Valid unspaced
root nodes or labelled root nodes might bypass this logic and incorrectly
evaluate to is_root = False.

[ ... ]
> @@ -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:
>                  continue

[Severity: Medium]
Is the c.is_root check reachable for overlay root nodes?

Overlay root nodes like "&{/} {" typically have c.node_name as None. Since
the check for c.node_name is None happens just before c.is_root, the loop
will already continue and the c.is_root check becomes dead code for them.

Also, since standard root nodes ("/") were previously skipped by the removed
c.node_name in (None, '/') check, and unspaced root nodes bypass the is_root
regex above, will they now be incorrectly subjected to the child-name
sorting logic?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803-n-dts-style-checker-continued-v3-0-6c9776928cea@oss.qualcomm.com?part=3