Re: [PATCH 1/1, RFC] Make Front Ends first class citizens

[Dodji Seketeli <dodji@redhat.com>]

Ben Woodard <woodard@redhat.com> writes:

> Overall sounds like a good logical design.

Thanks!

[...]


>> That class provides properties and behaviours that are shared by all
>> front-ends that libabigail supports today.  Namely, that class
>> provides access to (properties of) the ABI corpus, corpus groups,
>
> I follow you up to to the point where you get to corpus groups. It is
> not immediately obvious to me why corpus groups are included there. To 
> me corpus groups are some sort of container of corpuses (or corpora -
> if you speak latin) not something that lives inside corpus fe_iface.

That paragraph is poorly worded, at least.  I have updated it in the
newer version of the patch that I'll be posting soon.

What I wanted to say is that the ABI being built by the front-end is
accessible from the front end interface.

That ABI being built is either a corpus or a corpus group, depending on
the binary we are looking at.

In general, we are only interested in an ABI corpus.  Corpus groups are
for cases where we are looking at, for instance, an application that
loads plugins, like the Xorg server, the Gimp application or something
like the Apache web server or, the Linux Kernel (vmlinux + modules).

[...]

>> Lastly, there is an ABIXML front-end which class is named
>> abigail::abixml_reader::reader.  It inherits the abigail::fe_iface
>> class directly.  Note that unlike the two previous front-ends, this
>> one doesn't inherit the elf_based_reader base class, for reasons that
>> should be obvious to the astute reader by now.  So, this front-end
>> implements the abigail::fe_iface::load_corpus() abstract interface to
>> load the properties for the ABI corpus represented in the ABIXML
>> format, construct the internal representation and pass it to the
>> middle-end for further analysis.
>
> How about just calling it just abigail::abixml.

I ended up putting it into the abigail::abixml namespace, indeed.  It's
called abigail::abixml::reader now, in the next version of the patch.

> There is also a abixml writer backend, and I think that having all
> that code in in one place would be good.

I haven't re-organized backends, as I haven't thought about those yet.
I guess when people come up with, for instance, another ABI
serialization format that is worth supporting, the question of
supporting several of those will arise.

Thank you for mentioning that topic.

> Furthermore, abixml isn't always the most handy format. It is
> certainly not very compact. If CTF or BTF are all that they claim to
> be, then maybe having a binary file format using them would be more
> compact. The thing is to make CTF and BTF stand alone and usable by
> other tools you would need to have a simple ELF wrapper around them
> similar to the .gnu_reflink split DWARF format for debuginfo. So it
> would probably make sense to have abigail::elf_based_handler (renamed
> from elf_based_reader) and then have the classes which can write
> multiple inherit from another abstract class like abigail::be-iface
> which contained the writing functions. Having the reading and writing
> code in the same class would be a clean design.

Yeah, we will see.

[...]

Thanks a lot for your review.  I'll be sending out a new version of the
patchset with the update;

Cheers,

-- 
		Dodji