Re: [PATCH v6 01/10] dtc: dt-check-style: Handle sorting of top-level nodes and properties

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] _walk_bodies() fails to handle multiple node closures on a single line, corrupting the newly introduced node_stack and parent_dl state.
- [Low] Docstring for newly added _property_bucket_root() incorrectly claims 'device_type' is handled as a fixed slot.
--

commit dea26c9e2060b5f6b3d2cee7709c46d776cd4022
Author: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

dtc: dt-check-style: Handle sorting of top-level nodes and properties
    
Top-level DTS (but not example in the bindings) has only two nodes with
unit-addresses: memory@ and soc@.  There are two special cases here, in
terms of coding style.

> diff --git a/scripts/dtc/dt-check-style b/scripts/dtc/dt-check-style
> index 96deffc0d8a7c..5a9985baa0517 100755
> --- a/scripts/dtc/dt-check-style
> +++ b/scripts/dtc/dt-check-style
[ ... ]
> @@ -491,16 +496,24 @@ def _walk_bodies(lines):
>      """Yield lists of immediate-child NODE_OPEN lines for each node body
>      in the input. Skips ref-nodes (&label) since those don't have an
>      intrinsic ordering."""
> +    # Array of stacked nodes (parent/child)
>      body_stack = [[]]
> +    # Current stack of nodes, purely to track parent relationship for each node
> +    node_stack = []
> +    parent_dl = None
>      for dl in lines:
>          if dl.linetype == LineType.NODE_OPEN:
> +            dl.parent = parent_dl
> +            node_stack.append(dl)
>              body_stack[-1].append(dl)
>              body_stack.append([])
> +            parent_dl = dl
>              continue
>          if dl.linetype == LineType.NODE_CLOSE:
>              if len(body_stack) <= 1:
>                  # Unbalanced; ignore to avoid crashing on malformed input
>                  continue
> +            parent_dl = node_stack.pop().parent
>              yield body_stack.pop()
>              continue

[Severity: Medium]
Does _walk_bodies() correctly handle multiple node closures on a single line?

If a line contains multiple closures (such as }; };), it appears this block pops
node_stack exactly once per NODE_CLOSE line, regardless of dl.closures. This
under-pops the stack and permanently corrupts parent_dl. The newly introduced
c.parent.is_root evaluation will then be incorrect for subsequent nodes,
leading to false style warnings.

>      while body_stack:
[ ... ]
> @@ -591,6 +614,30 @@ def _property_bucket(name):
>      return (5 if ',' in stripped else 4, None)
>  
>  
> +def _property_bucket_root(name):
> +    """Return the canonical bucket index for a property:
> +       0 model (for root nodes only)
> +       1 compatible
> +    Plus a sub-key inside the bucket for fixed slots (device_type, compatible,
> +    reg, reg-names, ranges, status). 'standard' and 'vendor' return None for
> +    the sub-key, signalling that the within-bucket key is computed by
> +    the pairing rules."""
> +    stripped = name.lstrip('#')

[Severity: Low]
Is this docstring accurate regarding device_type?

The comment incorrectly lists device_type as a fixed slot for root nodes,
which contradicts the actual code in the function below since it does not check
for or handle device_type.

> +    if name == 'model':
> +        return (0, 0)
> +    if name == 'compatible':
> +        return (1, 0)
> +    if name == 'reg':
> +        return (2, 0)
> +    if name == 'reg-names':
> +        return (2, 1)
> +    if name == 'ranges':
> +        return (3, 0)
> +    if name == 'status':
> +        return (6, 0)
> +    return (5 if ',' in stripped else 4, None)
> +
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826-n-dts-style-checker-continued-v6-0-f270b1e81f8e@oss.qualcomm.com?part=1