[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 03/15] hw/ssi: Remove SSIBus from "qemu/typedefs
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 03/15] hw/ssi: Remove SSIBus from "qemu/typedefs.h" |
Date: |
Thu, 17 Jan 2019 10:03:24 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) |
Paolo Bonzini <address@hidden> 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.
- Re: [Qemu-devel] [PATCH 03/15] hw/ssi: Remove SSIBus from "qemu/typedefs.h", (continued)
- Re: [Qemu-devel] [PATCH 03/15] hw/ssi: Remove SSIBus from "qemu/typedefs.h", Thomas Huth, 2019/01/14
- Re: [Qemu-devel] [PATCH 03/15] hw/ssi: Remove SSIBus from "qemu/typedefs.h", Markus Armbruster, 2019/01/15
- Re: [Qemu-devel] [PATCH 03/15] hw/ssi: Remove SSIBus from "qemu/typedefs.h", Thomas Huth, 2019/01/15
- Re: [Qemu-devel] [PATCH 03/15] hw/ssi: Remove SSIBus from "qemu/typedefs.h", Paolo Bonzini, 2019/01/15
- Re: [Qemu-devel] [PATCH 03/15] hw/ssi: Remove SSIBus from "qemu/typedefs.h", Markus Armbruster, 2019/01/16
- Re: [Qemu-devel] [PATCH 03/15] hw/ssi: Remove SSIBus from "qemu/typedefs.h", Paolo Bonzini, 2019/01/16
- Re: [Qemu-devel] [PATCH 03/15] hw/ssi: Remove SSIBus from "qemu/typedefs.h", Gerd Hoffmann, 2019/01/16
- Re: [Qemu-devel] [PATCH 03/15] hw/ssi: Remove SSIBus from "qemu/typedefs.h", Paolo Bonzini, 2019/01/16
- Re: [Qemu-devel] [PATCH 03/15] hw/ssi: Remove SSIBus from "qemu/typedefs.h", Michael S. Tsirkin, 2019/01/16
- Re: [Qemu-devel] [PATCH 03/15] hw/ssi: Remove SSIBus from "qemu/typedefs.h", Markus Armbruster, 2019/01/17
- Re: [Qemu-devel] [PATCH 03/15] hw/ssi: Remove SSIBus from "qemu/typedefs.h",
Markus Armbruster <=
- Re: [Qemu-devel] [PATCH 03/15] hw/ssi: Remove SSIBus from "qemu/typedefs.h", Paolo Bonzini, 2019/01/15
- Re: [Qemu-devel] [PATCH 03/15] hw/ssi: Remove SSIBus from "qemu/typedefs.h", Markus Armbruster, 2019/01/16
[Qemu-devel] [PATCH 07/15] hw/bt: Remove HCIInfo from "qemu/typedefs.h", Philippe Mathieu-Daudé, 2019/01/11
[Qemu-devel] [PATCH 05/15] hw/nvram/fw_cfg: Remove various typedefs from "qemu/typedefs.h", Philippe Mathieu-Daudé, 2019/01/11
[Qemu-devel] [PATCH 04/15] hw/ide/ahci: Remove AllwinnerAHCIState from "qemu/typedefs.h", Philippe Mathieu-Daudé, 2019/01/11
[Qemu-devel] [PATCH 02/15] hw/pcmcia: Remove PCMCIACardState from "qemu/typedefs.h", Philippe Mathieu-Daudé, 2019/01/11