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

[Doug Anderson]

Hi David,

On Thu, Oct 29, 2026 at 8:55 PM David Gibson
<david@xxxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Wed, Oct 28, 2026 at 02:55:53PM -0700, Douglas Anderson wrote:
> > Add a `NodeTree` class to `pylibfdt` to provide an in-memory live tree
> > representation of a devicetree node and its subtree.
> >
> > While `FdtRo` / `Fdt` / `FdtSw` operate on flat binary blobs by node and
> > property offsets, tools that inspect, compare, or transform whole device
> > trees are much simpler when manipulating a Python tree of nodes and
> > properties.
> >
> > `NodeTree` provides:
> > - `NodeTree.unflatten(fdt, offset=0)` and `tree.flatten(...)` to convert
> >   between `FdtRo` blobs and `NodeTree` hierarchies.
> > - `get_node(path)`, `create_path(path)`, and `walk(path='')` for path
> >   navigation and pre-order traversal.
> > - Typed property getters and setters (`get_str()`, `set_str()`,
> >   `get_stringlist()`, `set_stringlist()`, `get_u32()`, `set_u32()`,
> >   `get_u32_list()`, `set_u32_list()`).
> >
> > Also fix `Property.as_str()` and `Property.as_stringlist()` when called
> > on an empty `b''` property so `as_str()` raises `ValueError` instead of
> > `IndexError` and `as_stringlist()` returns `[]`.
>
> Thanks for splitting this out as suggested.  However, on
> consideration, I don't think it really belongs in pylibfdt: the "f" in
> there refers to "flattened" and this is, explicitly, a non-flattened
> data structure.  I have sometimes thought about adding a library for
> handling non-flattened trees, although I wasn't thinking something
> Python specific.  I think for now let's move this to its own Python
> file / module.

Done. I've moved it out of `pylibfdt/libfdt.i` into its own `devicetree.py`
module.


> > diff --git a/pylibfdt/libfdt.i b/pylibfdt/libfdt.i
> > index 09fa5fb9ef3c..874a03a3c749 100644
> > --- a/pylibfdt/libfdt.i
> > +++ b/pylibfdt/libfdt.i
> > @@ -929,14 +929,16 @@ class Property(bytearray):
> >
> >      def as_str(self):
> >          """Unicode is supported by decoding from UTF-8"""
> > -        if self[-1] != 0:
> > +        if not self or self[-1] != 0:
> >              raise ValueError('Property lacks nul termination')
> >          if 0 in self[:-1]:
> >              raise ValueError('Property contains embedded nul characters')
> >          return self[:-1].decode('utf-8')
> >
> >      def as_stringlist(self):
> >          """Unicode is supported by decoding from UTF-8"""
> > +        if not self:
> > +            return []
> >          if self[-1] != 0:
> >              raise ValueError('Property lacks nul termination')
> >          parts = self[:-1].split(b'\x00')
>
> The two hunks above are good fixes, but unrelated to the bulk of the
> patch.

Split out into a separate patch ("pylibfdt: Handle empty properties in
as_str() and as_stringlist()") with direct unit tests for `Property`.


> > @@ -1214,6 +1216,261 @@ class NodeAdder():
> >          self._fdt.end_node()
> >
> >
> > +class NodeTree:
>
> Maybe class DeviceTree?  The "node" doesn't really add any useful
> information.

Done. Renamed to `DeviceTree` (in `devicetree.py`).


> > +    @classmethod
> > +    def unflatten(cls, fdt, offset=0):
> > +        """Unflatten an FdtRo subtree starting at `offset` into a NodeTree.
> > +
> > +        Args:
> > +            fdt: An `FdtRo` (or `Fdt`) instance.
> > +            offset: Starting node offset (default 0 for the root node).
> > +
> > +        Returns:
> > +            A `NodeTree` representing the node at `offset` and its descendants.
> > +        """
> > +        node = cls()
> > +        for prop in fdt.properties(offset):
> > +            node.props[prop.name] = bytes(prop)
>
> Since we already rely on class Property from libfdt.i, it seems like
> it would be a little more natural to have the dictionary values be
> class Property instances.  Yes, that technically has some redundant
> storage of the property names, but we're not particularly concerned
> about optimisation for this Python library.  Plus, Python's
> assign-by-reference should mean it's generally just a duplicated
> pointer, not a whole duplicated string.

Done. `node.props` now maps property names to `libfdt.Property` instances,
which also simplifies `get_str()`, `get_stringlist()`, `get_u32()`, and
`get_u32_list()`.


> > +        fdt = sw.as_fdt()
> > +        fdt_set_boot_cpuid_phys(fdt._fdt, boot_cpuid_phys)
> > +        fdt.pack()
> > +        return bytes(fdt.as_bytearray())
>
> For better symmetry with unflatten(), I'd suggest returning the Fdt

Done. `flatten()` now returns the packed `libfdt.Fdt` instance.


-Doug