[PATCH RFC] binman: Handle some newer pylint warnings

[Tom Rini]

On Tue, Feb 18, 2025 at 05:04:18PM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On Tue, 18 Feb 2025 at 12:25, Tom Rini <trini at konsulko.com> wrote:
> >
> > 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.
> 
> There is no need to disable these warnings so far as I can tell and it
> sets a bad precedent. We should just fix the code. I can send a patch
> for this if you like?

Yes, if you can fix the code please do as I don't see how to and they
need to be disabled for pylint_err to pass with current pylint versions,
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/9a163964/attachment.sig>