Re: [Qemu-devel] [PATCH 03/15] hw/ssi: Remove SSIBus from "qemu/typedefs.h"

[Markus Armbruster <armbru@redhat.com>]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 16/01/19 09:33, Markus Armbruster wrote:
>> What problem exactly are we trying to solve here?
>> To the best of my knowledge, typedefs.h has been working just fine for
>> us, and I can't see why it shouldn't continue to work just fine.
>
> Patches don't have to solve problems.  They can just help long-term
> maintainability, highlight code smells, etc.  Nobody is saying
> typedefs.h doesn't work.  But it's a tool, and including headers from
> headers is also a tool.  Each tool has its purpose and it is useful to
> question what are the exact purposes of the tools.
>
> typedefs.h is useful to avoid rebuilding the world too often if a type
> is used many times as a pointer, but rarely as a struct and rarely has
> functions called on its instances.  This is already a relatively rare
> case, but here we're talking about files that are included less than 20
> times; many of which also already include hw/ssi/ssi.h for their own
> business.  So we're hardly rebuilding anything more often, also because
> hw/ssi/ssi.h is not really changing fast.
>
> typedefs.h is also useful to avoid circular header inclusions.  Here
> hw/ssi/ssi.h is clearly a toplevel include for the SSI subsystem.  I
> agree that includes in a headers _can_ be painful, but sometimes they
> also tell you a hierarchy of what uses what.  typedefs.h doesn't;
> forward struct declarations also do, and I all but dislike using them in
> headers (Thomas, can you send v2 of the HACKING patch?).
>
> Therefore, typedefs.h is not particularly useful for SSIBus.  It doesn't
> hurt much, if at all, to leave it in.  On the other hand, it also
> doesn't hurt much if at all to remove it; it makes the SSI code a very
> tiny little bit more self contained.
>
> It may be that it's not particularly useful just because SSI is small;
> for example I2CBus is also in typedefs.h and it's used a lot more, maybe
> one day SSIBus will be the same.  In doubt, let's drop the patch---but I
> think it's useful to have the discussion and there are cases that are
> not controversial at all in Philippe's series.

I fully agree having the discussion is useful, and I also like many of
the patches in Philippe's series.