[diffoscope] [PATCH] Implement ElfFile as a container of sections

[Jérémy Bobbio]

Dhole:
> I'm attaching a patch to implement Container behaviour for elf files, so
> show the sections individually. I've also modified the test files to
> match the changes in the output.

Great work! :)

> If there is any issue, comment or improvement let me know so that I can
> address it :)

Yes, the code could be made nicer and more extensible quite easily.

> +_string_sections = ['.interp', '.dynstr', '.comment', '.debug_str']
> +
> +class ElfContainer(Container):
> +    def __init__(self, *args, **kwargs):
> +        super().__init__(*args, **kwargs)
> +        logger.debug('Creating ElfContainer %s', self.source.path)
> +        cmd = ['objdump', '--section-headers', self.source.path]

I think using `readelf --section-headers` would be better suited,
because you get a TYPE column. Then you could have a dictionnary from
TYPE to Python class. It would replace the `_string_sections` list and
that if series:

> +            if 'CODE' in section_type:
> +                self._sections[section_name] = ElfCodeSection
> +                self._section_list.append(section_name)
> +                logger.debug('Adding CODE section %s', section_name)
> +            elif section_name in _string_sections:
> +                self._sections[section_name] = ElfStringSection
> +                self._section_list.append(section_name)
> +                logger.debug('Adding String section %s', section_name)
> +            elif 'CONTENTS' in section_type:
> +                self._sections[section_name] = ElfSection
> +                self._section_list.append(section_name)
> +                logger.debug('Adding CONTENT section %s', section_name)
> +            else:
> +                # Some sections are empty. Eg.: .bss
> +                pass

When you start cascading ifs like that, it's a good thing to always ask
yourself: could I replace them by a dispatch table?

Another thing: as soon as we start interpreting output like that,
there's a chance we might be given things not exactly like we want them.
Could you make sure that diffoscope will not crash (but rather fallback
to binary diff for example) in case we can't parse the output of
readelf?

-- 
Lunar                                .''`. 
lunar at debian.org                    : :Ⓐ  :  # apt-get install anarchism
                                    `. `'` 
                                      `-   
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.reproducible-builds.org/pipermail/diffoscope/attachments/20151224/0ebb55cb/attachment.sig>