Re: [PATCH v2 02/10] pylibfdt: Add subnode/property iterators and get_mem_rsv_list()

[Simon Glass]

Hi Doug,

On 2026-10-28T21:46:49, Douglas Anderson <dianders@xxxxxxxxxxxx> wrote:
> pylibfdt: Add subnode/property iterators and get_mem_rsv_list()
>
> Add Pythonic iterator helpers on `FdtRo` to simplify walking nodes,
> properties, and memory reservation entries without manual offset loops
> or `-FDT_ERR_NOTFOUND` sentinel handling:
>
> - `subnodes(nodeoffset)`: yields the offset of each direct child subnode
>   of `nodeoffset`.
> - `properties(nodeoffset)`: yields a `Property` object for each property
>   of `nodeoffset`.
> - `get_mem_rsv_list()`: returns a list of `(address, size)` tuples for
>   all entries in the memory reservation block.
>
> Assisted-by: Gemini:gemini-pro-prerelease
> Signed-off-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
>
>  pylibfdt/libfdt.i       | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/pylibfdt_tests.py | 18 ++++++++++++++++++
>  2 files changed, 63 insertions(+)

> diff --git a/pylibfdt/libfdt.i b/pylibfdt/libfdt.i
> @@ -201,6 +201,23 @@ class FdtRo(object):
> +        soff = self.first_subnode(nodeoffset, quiet=QUIET_NOTFOUND)
> +        while soff >= 0:
> +            yield soff
> +            soff = self.next_subnode(soff, quiet=QUIET_NOTFOUND)

Since Fdt inherits this generator, a caller can easily write 'for off
in fdt.subnodes(0): fdt.del_node(off)' and get garbage, because the
offsets move under it. Since David has applied this, please can you
send a follow-up noting in the docstring (here and in properties())
that the tree must not be modified while iterating, as with
fdt_for_each_subnode() in C?

> diff --git a/tests/pylibfdt_tests.py b/tests/pylibfdt_tests.py
> @@ -243,6 +243,22 @@ class PyLibfdtBasicTests(unittest.TestCase):
> +        leaf = self.fdt.path_offset('/subnode@1/ss1')
> +        self.assertEqual(list(self.fdt.subnodes(leaf)), [])

The docstrings say both iterators raise FdtException for a bad offset,
but nothing tests that. A test that list(self.fdt.subnodes(-1)) and
list(self.fdt.properties(-1)) raise, perhaps alongside
testFirstNextSubnodeOffsetExceptions, would be good, perhaps in the
same follow-up. Since these are generators, the exception only appears
on the first next(), not when the method is called.

Reviewed-by: Simon Glass <sjg@xxxxxxxxxxxx>