Re: [PATCH v2 14/25] binman: Allow mkimage to use a non-zero fake-blob size

Simon Glass
2022-03-06T03:10:59Z

Hi Alper,

On Thu, 3 Mar 2022 at 14:16, Alper Nebi Yasak <alpernebiyasak@gmail.com> wrote:
>
> On 24/02/2022 02:00, Simon Glass wrote:
> > Unfortunately mkimage gets upset with zero-sized files. Update the
> > ObtainContents() method to support specifying the size, if a fake blob is
> > created.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> > Changes in v2:
> > - Add a patch to allow mkimage to use a non-zero fake-blob size
> >
> > tools/binman/entry.py | 11 ++++++++---
> > tools/binman/etype/_testing.py | 2 +-
> > tools/binman/etype/blob.py | 5 +++--
> > tools/binman/etype/mkimage.py | 13 ++++++++++++-
> > tools/binman/etype/section.py | 2 +-
> > tools/binman/ftest.py | 10 ++++++++++
> > tools/binman/test/229_mkimage_missing.dts | 18 ++++++++++++++++++
> > 7 files changed, 53 insertions(+), 8 deletions(-)
> > create mode 100644 tools/binman/test/229_mkimage_missing.dts
>
> I don't exactly like this, but can't think of a nice alternative either.
>
> Reviewed-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
>
> > [...]
> >
> > diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py
> > index 9f4717e4ea..5f6def2287 100644
> > --- a/tools/binman/etype/mkimage.py
> > +++ b/tools/binman/etype/mkimage.py
> > @@ -51,8 +51,9 @@ class Entry_mkimage(Entry):
> > self.ReadEntries()
> >
> > def ObtainContents(self):
> > + # Use a non-zero size for any fake files to keep mkimage happy
> > data, input_fname, uniq = self.collect_contents_to_file(
> > - self._mkimage_entries.values(), 'mkimage')
> > + self._mkimage_entries.values(), 'mkimage', 1024)
>
> I kind of want to say that mkimage-the-etype should be able to handle
> here whatever it gets from subentries (maybe by writing a single-byte
> file itself), and mkimage-the-executable should be able to handle
> zero-size files, but I'm not confident in those opinions.

Well the entry has no problem with missing files, so that should be OK.

For mkimage I agree it is a strange restriction. Perhaps we should
just change it? I don't see what problem it could create.

>
> > if data is None:
> > return False
> > output_fname = tools.get_output_filename('mkimage-out.%s' % uniq)
> > [...]

Regards,
Simon