[PATCH] binman: Correct the error message for a bad hash algorithm

[Simon Glass]

Hi Alper,

On Tue, 8 Feb 2022 at 11:54, Alper Nebi Yasak <alpernebiyasak at gmail.com> wrote:
>
> On 08/02/2022 20:59, Simon Glass wrote:
> > This shows an internal type at present, rather than the algorithm name.
> > Fix it and update the test to catch this.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
> >
> >  tools/binman/ftest.py | 2 +-
> >  tools/binman/state.py | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
>
> Reviewed-by: Alper Nebi Yasak <alpernebiyasak at gmail.com>
>
> I saw the failing build for my series [1]. Looks to me like binman
> doesn't support crc32 in hash nodes, and turning FIT into a Section
> simply exposed that. I tried a sloppy fix, see below.
>
> [1] https://source.denx.de/u-boot/custodians/u-boot-dm/-/jobs/388771
>
> > diff --git a/tools/binman/state.py b/tools/binman/state.py
> > index af0a65e841..843aefd28d 100644
> > --- a/tools/binman/state.py
> > +++ b/tools/binman/state.py
> > @@ -397,7 +397,7 @@ def CheckAddHashProp(node):
> >          if algo.value == 'sha256':
> >              size = 32
>
> If I add here:
>
>   +        elif algo.value == 'crc32':
>   +            size = 8
>
> and a crc32 implementation attempt to the next function:
>
> >          else:
> > -            return "Unknown hash algorithm '%s'" % algo
> > +            return "Unknown hash algorithm '%s'" % algo.value
> >          for n in GetUpdateNodes(hash_node):
> >              n.AddEmptyProp('value', size)
>
>    def CheckSetHashValue(node, get_data_func):
>        hash_node = node.FindNode('hash')
>        if hash_node:
>            algo = hash_node.props.get('algo').value
>            if algo == 'sha256':
>                m = hashlib.sha256()
>                m.update(get_data_func())
>                data = m.digest()
>   +        elif algo == 'crc32':
>   +            data = zlib.crc32(get_data_func()).to_bytes(8, 'little')
>            for n in GetUpdateNodes(hash_node):
>                n.SetData('value', data)
>
> the failing boards start building again. Not sure how correct this is
> though (especially the endianness). Do we need this and maybe a test
> with crc32 hash now?

I can get your series in without the final patch (testing at
u-boot-dm/testing now).

The thing is that mkimage creates the hashes so we don't want binman
doing that too.

My current feeling is that we need a way to tell sections not to add
calculated properties for hashes...but just for the FIT section
itself, so its children should still add these.

Regards,
Simon