[PATCH 3/5] binman: capsule: Use dumped capsule header contents for verification

[Sughosh Ganu]

hi Simon,

On Sun, 8 Oct 2023 at 04:41, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Sughosh,
>
> On Wed, 4 Oct 2023 at 05:27, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> >
> > The various fields of a generated capsule are currently verified
> > through hard-coded offsets. Use the dump-capsule feature for dumping
> > the capsule header contents and use those for capsule verification.
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> > ---
> >  tools/binman/ftest.py | 95 ++++++++++++++++++++++++-------------------
> >  1 file changed, 54 insertions(+), 41 deletions(-)
>
> This looks good apart from a few nits below. However, the tests fail for me.

Can you please specify which tests fail, and the way to reproduce the
failures? I ran the tests before sending the patches, and they ran
fine, including the coverage which is 100%. Ci too did not complain.

>
> >
> > diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> > index 8e419645a6..2b8871ab7e 100644
> > --- a/tools/binman/ftest.py
> > +++ b/tools/binman/ftest.py
> > @@ -121,9 +121,11 @@ COMP_BINTOOLS = ['bzip2', 'gzip', 'lz4', 'lzma_alone', 'lzop', 'xz', 'zstd']
> >  TEE_ADDR = 0x5678
> >
> >  # Firmware Management Protocol(FMP) GUID
> > -FW_MGMT_GUID = 'edd5cb6d2de8444cbda17194199ad92a'
> > +FW_MGMT_GUID = '6DCBD5ED-E82D-4C44-BDA1-7194199AD92A'
> >  # Image GUID specified in the DTS
> > -CAPSULE_IMAGE_GUID = '52cfd7092007104791d108469b7fe9c8'
> > +CAPSULE_IMAGE_GUID = '09D7CF52-0720-4710-91D1-08469B7FE9C8'
> > +# Windows cert GUID
> > +WIN_CERT_TYPE_EFI_GUID = '4AAFD29D-68DF-49EE-8AA9-347D375665A7'
>
> Please use lower-case hex

Okay

>
> >
> >  class TestFunctional(unittest.TestCase):
> >      """Functional tests for binman
> > @@ -7223,52 +7225,63 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
> >          self.assertRegex(err,
> >                           "Image 'image'.*missing bintools.*: bootgen")
> >
> > +    def _GetCapsuleHeaders(self, data):
>
> This should have a function comment so it is clear what it is doing, returning.

Will add

-sughosh

>
> > +        capsule_file = os.path.join(self._indir, 'test.capsule')
> > +        tools.write_file(capsule_file, data)
> > +
> > +        out = tools.run('mkeficapsule', '--dump-capsule', capsule_file)
> > +        lines = out.splitlines()
> > +
> > +        re_line = re.compile(r'^([^:\-\t]*)(?:\t*\s*:\s*(.*))?$')
> > +        vals = collections.defaultdict(list)
> > +        for line in lines:
> > +            mat = re_line.match(line)
> > +            if mat:
> > +                vals[mat.group(1)] = mat.group(2)
> > +
> > +        return vals
> > +
> [..]
>
> Regards,
> Simon