Hi Alper,
On Tue, 15 Feb 2022 at 04:53, Alper Nebi Yasak <alpernebiya...@gmail.com> wrote:
>
> On 08/02/2022 21:49, Simon Glass wrote:
> > Add a function which reads the segments and the entry address.
> >
> > Also fix a comment nit in the tests while we are here.
> >
> > Signed-off-by: Simon Glass <s...@chromium.org>
> > ---
> >
> > tools/binman/elf.py | 37 +++++++++++++++++++++++++++++++++++++
> > tools/binman/elf_test.py | 31 +++++++++++++++++++++++++++++--
> > 2 files changed, 66 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/binman/elf.py b/tools/binman/elf.py
> > index de2bb4651f..2b83ac1876 100644
> > --- a/tools/binman/elf.py
> > +++ b/tools/binman/elf.py
> > @@ -20,6 +20,7 @@ from patman import tout
> > ELF_TOOLS = True
> > try:
> > from elftools.elf.elffile import ELFFile
> > + from elftools.elf.elffile import ELFError
> > from elftools.elf.sections import SymbolTableSection
> > except: # pragma: no cover
> > ELF_TOOLS = False
> > @@ -369,3 +370,39 @@ def UpdateFile(infile, outfile, start_sym, end_sym,
> > insert):
> > newdata += data[syms[end_sym].offset:]
> > tools.WriteFile(outfile, newdata)
> > tout.Info('Written to offset %#x' % syms[start_sym].offset)
> > +
> > +def read_segments(data):
> > + """Read segments from an ELF file
> > +
> > + Args:
> > + data (bytes): Contents of file
> > +
> > + Returns:
> > + tuple:
> > + list of segments, each:
> > + int: Segment number (0 = first)
> > + int: Start address of segment in memory
> > + bytes: Contents of segment
> > + int: entry address for image
> > +
> > + Raises:
> > + ValueError: elftools is not available
>
> ... or input data is not a correct ELF file?
>
> > + """
> > + if not ELF_TOOLS:
> > + raise ValueError('Python elftools package is not available')
>
> I see something like ModuleNotFoundError("No module named 'elftools'")
> when I try to import an unavailable module, so maybe this could match that.
OK, will update.
>
> > + with io.BytesIO(data) as inf:
> > + try:
> > + elf = ELFFile(inf)
> > + except ELFError as err:
> > + raise ValueError(err)
>
> Could also be: raise ValueError("Not an ELF file") from err
>
> But I guess you want err's message here to match on it in tests. (It's
> also possible but slightly inconvenient with __cause__ when using
> raise-from)
It might be a corrupt ELF file, perhaps.
>
> > + entry = elf.header['e_entry']
> > + segments = []
> > + for i in range(elf.num_segments()):
> > + segment = elf.get_segment(i)
> > + if segment['p_type'] != 'PT_LOAD' or not segment['p_memsz']:
>
> I can't say I fully understand ELF details, is it obvious in context
> that a function named read_segments() would only return these segments,
> or should the name be explicit about it e.g. read_loadable_segments()?
Yes I agree.
[..]
Regards,
Simon
