[PATCH v2 01/11] binman: elf: Check for ELF_TOOLS availability and remove extra semicolon

[Lukas Funke]

Hi Simon,

On 07.07.2023 19:35, Simon Glass wrote:
> Hi Lukas,
> 
> On Thu, 6 Jul 2023 at 09:38, <lukas.funke-oss at weidmueller.com> wrote:
>>
>> From: Lukas Funke <lukas.funke at weidmueller.com>
>>
>> Check if elf tools are available when running DecodeElf(). Also
>> remove superfuous semicolon at line ending.
>>
>> Signed-off-by: Lukas Funke <lukas.funke at weidmueller.com>
>> Reviewed-by: Simon Glass <sjg at chromium.org>
>> ---
>>
>> (no changes since v1)
>>
>>   tools/binman/elf.py | 10 ++++++----
>>   1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/binman/elf.py b/tools/binman/elf.py
>> index 5816284c32..a53f4b9c4f 100644
>> --- a/tools/binman/elf.py
>> +++ b/tools/binman/elf.py
>> @@ -438,13 +438,15 @@ def DecodeElf(data, location):
>>       Returns:
>>           ElfInfo object containing information about the decoded ELF file
>>       """
>> +    if not ELF_TOOLS:
>> +        raise ValueError("Python: No module named 'elftools'")
> 
> Actually this is missing test coverage. See testEmbedFail() for an
> example of how to add it for this function.
> 
> Use 'binman test -T' to see test coverage.

I've extended the test in order to cover the other functions. But 
'LookupAndWriteSymbols' failed due to different error message:
"Cannot write symbols to an ELF file without Python elftools" instead of 
"Python: No module named 'elftools'"

Would your prefer to adapt the error message in 'LookupAndWriteSymbols' 
or add another 'assertIn' to cover the additional message? IMHO the 
error messages should be the same.

> 
>>       file_size = len(data)
>>       with io.BytesIO(data) as fd:
>>           elf = ELFFile(fd)
>> -        data_start = 0xffffffff;
>> -        data_end = 0;
>> -        mem_end = 0;
>> -        virt_to_phys = 0;
>> +        data_start = 0xffffffff
>> +        data_end = 0
>> +        mem_end = 0
>> +        virt_to_phys = 0
>>
>>           for i in range(elf.num_segments()):
>>               segment = elf.get_segment(i)
>> --
>> 2.30.2
>>
> 
> Regards,
> Simon