[PATCH RFC] binman: Handle some newer pylint warnings

[Tom Rini]

On Wed, Feb 12, 2025 at 04:22:59PM -0600, Tom Rini wrote:

> With a newer pylint we have a few warnings show up now that are harder
> to handle. The first problem is that when we call self.Raise(...) this
> is no longer enough to have pylint see that we're now bailing out and
> won't attempt to reference an unassigned variable. This is all of the
> pylint disabling, with the exception of _AddNode.
> 
> The problem with _AddNode is that before we call this function we set
> fsw as libfdt.FdtSw() and I'm unsure how best to rework the code.
> 
> The final problem is in CheckSetHashValue() where the function only
> handles one algorithm but the intention is to handle others as well.
> Once another algorithm is added and then also error handling for no
> algorithm is added, we would not attempt to use data before it's been
> assigned.
> 
> As I'm not confident all of these are the right way to handle the
> problem I've marked this as RFC and not split the changes up more.
> 
> Signed-off-by: Tom Rini <trini at konsulko.com>
> ---
> Cc: Simon Glass <sjg at chromium.org>
> ---
>  tools/binman/etype/fdtmap.py       | 1 +
>  tools/binman/etype/image_header.py | 1 +
>  tools/binman/etype/pre_load.py     | 1 +
>  tools/binman/etype/x509_cert.py    | 1 +
>  tools/binman/ftest.py              | 1 +
>  tools/binman/state.py              | 4 ++--
>  6 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/binman/etype/fdtmap.py b/tools/binman/etype/fdtmap.py
> index f1f6217940f2..a2b0d27fcc5f 100644
> --- a/tools/binman/etype/fdtmap.py
> +++ b/tools/binman/etype/fdtmap.py
> @@ -109,6 +109,7 @@ class Entry_fdtmap(Entry):
>          def _AddNode(node):
>              """Add a node to the FDT map"""
>              for pname, prop in node.props.items():
> +                # pylint: disable=locally-disabled, possibly-used-before-assignment
>                  fsw.property(pname, prop.bytes)
>              for subnode in node.subnodes:
>                  with fsw.add_node(subnode.name):
> diff --git a/tools/binman/etype/image_header.py b/tools/binman/etype/image_header.py
> index 240118849580..6d8db9eed564 100644
> --- a/tools/binman/etype/image_header.py
> +++ b/tools/binman/etype/image_header.py
> @@ -72,6 +72,7 @@ class Entry_image_header(Entry):
>              image_size = self.section.GetImageSize() or 0
>              base = (0 if self.location != 'end' else image_size)
>              offset = (image_pos - base) & 0xffffffff
> +        # pylint: disable=locally-disabled, possibly-used-before-assignment
>          data = IMAGE_HEADER_MAGIC + struct.pack('<I', offset)
>          return data
>  
> diff --git a/tools/binman/etype/pre_load.py b/tools/binman/etype/pre_load.py
> index 2e4c72359ff3..e2615e1c7d1b 100644
> --- a/tools/binman/etype/pre_load.py
> +++ b/tools/binman/etype/pre_load.py
> @@ -122,6 +122,7 @@ class Entry_pre_load(Entry_collection):
>          else:
>              self.Raise(padding_name + " is not supported")
>  
> +        # pylint: disable=locally-disabled, possibly-used-before-assignment
>          sig = padding.new(key, **padding_args).sign(hash_image)
>  
>          hash_sig = SHA256.new()
> diff --git a/tools/binman/etype/x509_cert.py b/tools/binman/etype/x509_cert.py
> index 29630d1b86c8..15d0766051c2 100644
> --- a/tools/binman/etype/x509_cert.py
> +++ b/tools/binman/etype/x509_cert.py
> @@ -141,6 +141,7 @@ class Entry_x509_cert(Entry_collection):
>                  dm_data_ext_boot_block=self.dm_data_ext_boot_block,
>                  bootcore_opts=self.bootcore_opts
>              )
> +        # pylint: disable=locally-disabled, possibly-used-before-assignment
>          if stdout is not None:
>              data = tools.read_file(output_fname)
>          else:
> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> index a553ca9e5642..b6e58decb008 100644
> --- a/tools/binman/ftest.py
> +++ b/tools/binman/ftest.py
> @@ -6387,6 +6387,7 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
>                      expect_val = entry.image_pos
>                  elif prop_name == 'size':
>                      expect_val = entry.size
> +                # pylint: disable=locally-disabled, possibly-used-before-assignment
>                  self.assertEqual(expect_val, val)
>  
>      def testSymbolsElfBad(self):
> diff --git a/tools/binman/state.py b/tools/binman/state.py
> index 45bae40c525a..ca3f857f1943 100644
> --- a/tools/binman/state.py
> +++ b/tools/binman/state.py
> @@ -410,8 +410,8 @@ def CheckSetHashValue(node, get_data_func):
>              m = hashlib.sha256()
>              m.update(get_data_func())
>              data = m.digest()
> -        for n in GetUpdateNodes(hash_node):
> -            n.SetData('value', data)
> +            for n in GetUpdateNodes(hash_node):
> +                n.SetData('value', data)
>  
>  def SetAllowEntryExpansion(allow):
>      """Set whether post-pack expansion of entries is allowed

Any thoughts on this Simon? Moving to a newer Ubuntu host for CI is
blocked on moving to a newer pylint. Thanks.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20250218/e9a54230/attachment.sig>