Re: [PATCH v3 03/15] pylibfdt: Clarify iterator docstrings and test negative offsets

[David Gibson]

On Sun, Nov 01, 2026 at 02:01:10PM -0800, Douglas Anderson wrote:
> Follow-up improvements to commit 6b0371afc532 ("pylibfdt: Add
> subnode/property iterators and get_mem_rsv_list()"):
> 
> - Document in `FdtRo.subnodes()` and `FdtRo.properties()` that the
>   device tree must not be modified while iterating, as node or property
>   offsets may shift.
> - Remove the inaccurate `Raises:` section from `FdtRo.subnodes()`, as
>   `fdt_first_subnode()` and `fdt_next_subnode()` map all negative
>   returns from `fdt_next_node()` to `-FDT_ERR_NOTFOUND` (which
>   `subnodes()` quiets).
> - Add a test case in `tests/pylibfdt_tests.py` verifying that passing a
>   negative offset (`-1`) to `properties()` raises `FdtException` with
>   `-libfdt.BADOFFSET`.
> 
> Assisted-by: Gemini:gemini-pro-prerelease
> Signed-off-by: Douglas Anderson <dianders@xxxxxxxxxxxx>

Reviewed-by: David Gibson <david@xxxxxxxxxxxxxxxxxxxxx>

> ---
> 
> Changes in v3:
> - New follow-up patch for already-applied commit 6b0371afc532
>   ("pylibfdt: Add subnode/property iterators and get_mem_rsv_list()")
>   clarifying iterator docstrings and testing negative offsets (Simon
>   Glass, David Gibson)
> 
>  pylibfdt/libfdt.i       | 9 ++++++---
>  tests/pylibfdt_tests.py | 4 ++++
>  2 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/pylibfdt/libfdt.i b/pylibfdt/libfdt.i
> index 0db7110b4a38..5c8859b37df3 100644
> --- a/pylibfdt/libfdt.i
> +++ b/pylibfdt/libfdt.i
> @@ -204,14 +204,14 @@ class FdtRo(object):
>      def subnodes(self, nodeoffset):
>          """Iterate over the subnodes of a node
>  
> +        The device tree must not be modified while iterating, as node offsets
> +        may shift.
> +
>          Args:
>              nodeoffset: Offset of parent node to check
>  
>          Yields:
>              Node offset of each direct subnode in order
> -
> -        Raises:
> -            FdtException on error (e.g. invalid nodeoffset)
>          """
>          soff = self.first_subnode(nodeoffset, quiet=QUIET_NOTFOUND)
>          while soff >= 0:
> @@ -465,6 +465,9 @@ class FdtRo(object):
>      def properties(self, nodeoffset):
>          """Iterate over the properties of a node
>  
> +        The device tree must not be modified while iterating, as property
> +        offsets may shift.
> +
>          Args:
>              nodeoffset: Offset of node to check
>  
> diff --git a/tests/pylibfdt_tests.py b/tests/pylibfdt_tests.py
> index 2a0509c7802e..93408745c575 100644
> --- a/tests/pylibfdt_tests.py
> +++ b/tests/pylibfdt_tests.py
> @@ -171,6 +171,10 @@ class PyLibfdtBasicTests(unittest.TestCase):
>              self.fdt.first_property_offset(node)
>          self.assertEqual(e.exception.err, -libfdt.NOTFOUND)
>  
> +        with self.assertRaises(FdtException) as e:
> +            list(self.fdt.properties(-1))
> +        self.assertEqual(e.exception.err, -libfdt.BADOFFSET)
> +
>      def testGetName(self):
>          """Check that we can get the name of a node"""
>          self.assertEqual(self.fdt.get_name(0), '')
> -- 
> 2.56.0.317.g454f8302e1-goog
> 
> 

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

Attachment: signature.asc
Description: PGP signature