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. Paolo