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

[Simon Glass]

Hi Sughosh,

On Mon, 9 Oct 2023 at 01:46, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
>
> 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.

Sure, for example:

$ binman test testCapsuleGen
======================== Running binman tests ========================
/usr/lib/python3.10/os.py:1030: RuntimeWarning: line buffering
(buffering=1) isn't supported in binary mode, the default buffer size will
be used
  return io.open(fd, mode, buffering, encoding, *args, **kwargs)
/usr/lib/python3.10/os.py:1030: RuntimeWarning: line buffering
(buffering=1) isn't supported in binary mode, the default buffer size will
be used
  return io.open(fd, mode, buffering, encoding, *args, **kwargs)
F
======================================================================
FAIL: binman.ftest.TestFunctional.testCapsuleGen (subunit.RemotedTestCase)
binman.ftest.TestFunctional.testCapsuleGen
----------------------------------------------------------------------
testtools.testresult.real._StringException: Traceback (most recent call
last):
AssertionError: '6DCBD5ED-E82D-4C44-BDA1-7194199AD92A' != []


----------------------------------------------------------------------
Ran 1 test in 0.147s

FAILED (failures=1)


>
> >
> > >
> > > 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

Regards,
SImon