On 11.09.2017 12:23, Paolo Bonzini wrote: > On 10/09/2017 00:07, Eduardo Habkost wrote: >> On Fri, Sep 08, 2017 at 02:46:36PM +0200, David Hildenbrand wrote: >>> On 08.09.2017 06:21, Thomas Huth wrote: >>>> On 07.09.2017 22:13, David Hildenbrand wrote: >>>>> Implemented in sclp.c, so let's move it to the right include file. >>>>> Fix up one include. Do a forward declaration of CPUS390XState to fix the >>>>> two sclp consoles complaining. >>>>> >>>>> Signed-off-by: David Hildenbrand <da...@redhat.com> >>>>> --- >>>>> include/hw/s390x/sclp.h | 2 ++ >>>>> target/s390x/cpu.h | 1 - >>>>> target/s390x/misc_helper.c | 1 + >>>>> 3 files changed, 3 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h >>>>> index a72d096081..4b86a8a293 100644 >>>>> --- a/include/hw/s390x/sclp.h >>>>> +++ b/include/hw/s390x/sclp.h >>>>> @@ -242,5 +242,7 @@ sclpMemoryHotplugDev >>>>> *init_sclp_memory_hotplug_dev(void); >>>>> sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void); >>>>> void sclp_service_interrupt(uint32_t sccb); >>>>> void raise_irq_cpu_hotplug(void); >>>>> +typedef struct CPUS390XState CPUS390XState; >>>>> +int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code); >>>> >>>> That's dangerous and likely does not work with certain versions of GCC. >>>> You can't do a "forward declaration" with typedef in C, I'm afraid. See >>>> for example: >>>> >>>> https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg01454.html >>>> https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg03337.html >>>> https://stackoverflow.com/questions/8367646/redefinition-of-typedef >>>> >>>> All this typedef'ing in QEMU is pretty bad ... we run into this problem >>>> again and again. include/qemu/typedefs.h is just a work-around for this. >>>> I know people like typedefs for some reasons (I used to do that, too, >>>> before I realized the trouble with them), but IMHO we should rather >>>> adopt the typedef-related rules from the kernel coding conventions instead: >>>> >>>> https://www.kernel.org/doc/html/v4.13/process/coding-style.html#typedefs >>>> >>>> Thomas >>>> >>> >>> Yes, this is really nasty. And I wasn't aware of the involved issues. >>> >>> This seems to be the only feasible solution (including cpu.h sounds >>> wrong and will require a bunch of other includes): >>> >>> >>> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h >>> index a72d096081..ce80915a02 100644 >>> --- a/include/hw/s390x/sclp.h >>> +++ b/include/hw/s390x/sclp.h >>> @@ -242,5 +242,7 @@ sclpMemoryHotplugDev >>> *init_sclp_memory_hotplug_dev(void); >>> sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void); >>> void sclp_service_interrupt(uint32_t sccb); >>> void raise_irq_cpu_hotplug(void); >>> +struct CPUS390XState; >>> +int sclp_service_call(struct CPUS390XState *env, uint64_t sccb, >>> uint32_t code); >>> >>> #endif >> >> Why not use typedefs.h? > > See Markus's reply. But, maybe it's even better to use S390CPU* and > include target/s390x/cpu-qom.h, which by design provides as little > definitions as needed.
I'll go with that approach. I'll drop the dependency from cpu-qom.h to cpu_models.h (by moving typedefs to cpu-qom.h). This makes it compile at hopefully should then be good enough for now. (this approach implies dropping patch "[PATCH v3 05/21] target/s390x: move typedef of S390CPU to its definition"). Thanks! > >> Signed-off-by: Eduardo Habkost <ehabk...@redhat.com> -- Thanks, David