Re: [PATCH v3 04/15] pylibfdt: Handle empty properties in as_str() and as_stringlist()

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3545 bytes --]

On Sun, Nov 01, 2026 at 02:01:11PM -0800, Douglas Anderson wrote:
> Currently, calling `Property.as_str()` or `Property.as_stringlist()` on
> an empty `Property` (`b''`) raises `IndexError: bytearray index out of
> range` when indexing `self[-1]`.
> 
> Update `Property.as_str()` to raise `ValueError('Property lacks nul
> termination')` on an empty property, and update
> `Property.as_stringlist()` to return `[]` on an empty property. Add unit
> tests in `tests/pylibfdt_tests.py`.
> 
> Assisted-by: Gemini:gemini-pro-prerelease
> Signed-off-by: Douglas Anderson <dianders@chromium.org>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
> 
> Changes in v3:
> - Split Property.as_str() and Property.as_stringlist() empty-property
>   handling into a standalone patch and add direct Property unit tests
>   (Simon Glass, David Gibson)
> 
>  pylibfdt/libfdt.i       | 4 +++-
>  tests/pylibfdt_tests.py | 7 +++++++
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/pylibfdt/libfdt.i b/pylibfdt/libfdt.i
> index 5c8859b37df3..eb14312986ea 100644
> --- a/pylibfdt/libfdt.i
> +++ b/pylibfdt/libfdt.i
> @@ -932,7 +932,7 @@ 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')
> @@ -940,6 +940,8 @@ class Property(bytearray):
>  
>      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')
> diff --git a/tests/pylibfdt_tests.py b/tests/pylibfdt_tests.py
> index 93408745c575..20741d8f2df0 100644
> --- a/tests/pylibfdt_tests.py
> +++ b/tests/pylibfdt_tests.py
> @@ -423,6 +423,10 @@ class PyLibfdtBasicTests(unittest.TestCase):
>                           self.fdt.getprop(node, "compatible").as_stringlist())
>          self.assertEqual(["this is a placeholder string", "string2"],
>                           self.fdt.getprop(node, "placeholder").as_stringlist())
> +        self.assertEqual([], libfdt.Property('empty', b'').as_stringlist())
> +        with self.assertRaises(ValueError) as e:
> +            libfdt.Property('bad', b'no-nul').as_stringlist()
> +        self.assertIn('lacks nul termination', str(e.exception))
>  
>      def testReserveMap(self):
>          """Test that we can access the memory reserve map"""
> @@ -523,6 +527,9 @@ class PyLibfdtBasicTests(unittest.TestCase):
>          with self.assertRaises(ValueError) as e:
>              self.fdt.getprop(node, 'prop-int').as_str()
>          self.assertIn('lacks nul termination', str(e.exception))
> +        with self.assertRaises(ValueError) as e:
> +            libfdt.Property('empty', b'').as_str()
> +        self.assertIn('lacks nul termination', str(e.exception))
>  
>          node2 = self.fdt.path_offset('/subnode@1/subsubnode')
>          with self.assertRaises(ValueError) as e:
> -- 
> 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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]