Re: [PATCH v2 03/10] pylibfdt: Add NodeTree live tree representation

[Simon Glass]

Hi Doug,

On 2026-10-28T21:46:49, Douglas Anderson <dianders@xxxxxxxxxxxx> wrote:
> pylibfdt: Add NodeTree live tree representation
>
> Add `NodeTree`, an in-memory live tree representation of a devicetree
> node and its subtree, to `pylibfdt`:
>
> - `NodeTree.unflatten(fdt, offset=0)` builds a `NodeTree` hierarchy from
>   an `FdtRo` instance.
> - `NodeTree.flatten(mem_rsv=(), boot_cpuid_phys=0)` serializes a root
>   `NodeTree` back into a packed FDT blob via `FdtSw`.
> - `get_node()`, `create_path()`, and `walk()` provide path lookup,
>   intermediate node creation, and pre-order traversal.
> - `get_str()`, `set_str()`, `get_stringlist()`, `set_stringlist()`,
>   `get_u32()`, `set_u32()`, `get_u32_list()`, and `set_u32_list()`
>   provide typed property access and mutation (updating
>   `Property.as_str()` and `Property.as_stringlist()` so empty `b''`
>   properties raise `ValueError` or return `[]` rather than
>   `IndexError`).
>
> Add unit tests in `tests/pylibfdt_tests.py` covering `NodeTree`
> unflatten/flatten round-tripping and helper methods.
> [...]
>
>  pylibfdt/libfdt.i       | 262 +++++++++++++++++++++++++++++++++++++++++++++++-
>  tests/pylibfdt_tests.py |  54 ++++++++++
>  2 files changed, 315 insertions(+), 1 deletion(-)

> provide typed property access and mutation (updating
> `Property.as_str()` and `Property.as_stringlist()` so empty `b''`
> properties raise `ValueError` or return `[]` rather than
> `IndexError`).

When you split this out (as David suggests), please can you add tests
that call Property directly rather than only going through NodeTree?

> diff --git a/pylibfdt/libfdt.i b/pylibfdt/libfdt.i
> @@ -1210,6 +1212,264 @@ class NodeAdder():
> +        node = cls()
> +        for prop in fdt.properties(offset):
> +            node.props[prop.name] = bytes(prop)
> +        for suboff in fdt.subnodes(offset):
> +            node.children[fdt.get_name(suboff)] = cls.unflatten(fdt, suboff)
> +        return node

If a blob has two sibling nodes (or two properties) with the same
name, one is silently dropped. I checked with FdtSw: with two 'a'
subnodes under the root, only the second survives. libfdt does not
reject such blobs and fdtfactor reads input it does not control, so I
think it is better to raise ValueError here than to quietly throw data
away.

> diff --git a/pylibfdt/libfdt.i b/pylibfdt/libfdt.i
> @@ -1210,6 +1212,264 @@ class NodeAdder():
> +        self._flatten_subtree(sw, '')
> +        fdt = sw.as_fdt()
> +        fdt_set_boot_cpuid_phys(fdt._fdt, boot_cpuid_phys)
> +        fdt.pack()

This reaches into fdt._fdt and calls the raw SWIG function. It would
be worth adding Fdt.set_boot_cpuid_phys() to go with the existing
FdtRo.boot_cpuid_phys(), and calling that instead.

> diff --git a/pylibfdt/libfdt.i b/pylibfdt/libfdt.i
> @@ -1210,6 +1212,264 @@ class NodeAdder():
> +class NodeTree:
> +    """In-memory live tree representation of a devicetree node and its subtree.
> +
> +    Attributes:
> +        props: Dict mapping property name (str) to raw property value (bytes).
> +        children: Dict mapping child node name (str) to child NodeTree.
> +    """

BTW in patch 7, fdtfactor adds 'phandle' and 'fixups' attributes to
NodeTree objects on the fly, so they carry attributes the class does
not declare. Since unflatten() uses cls and create_path() uses
type(self), subclassing already works, so a small subclass in
fdtfactor declaring these two attributes would be cleaner. What do you
think?

Regards,
Simon