On 08.09.2017 14:29, Markus Armbruster wrote: > Thomas Huth <th...@redhat.com> writes: > >> 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 > > I prefer the kernel style for typedefs myself. But it's strictly a > matter of style. Excessive typedeffing makes code harder to understand, > it isn't wrong. The part that's wrong is defining things more than > once, and that applies to everything, not just typedefs. > > Sometimes you get away with defining something more than once. It's > still wrong. > > You're not supposed to define the same variable more than once, either > (although many C compilers let you get away with it, see -fno-common). > You define it in *one* place. If you need to declare it, declare it in > *one* place, and make sure that place is in scope at the definition, so > the compiler can check the two match. > > Likewise, you're not supposed to define the same struct tag more than > once, and you should declare it in just one place.
AFAIK it's perfect valid C to do a forward declaration of a struct multiple times by just writing e.g. "struct CPUS390XState;" somewhere in your code. This is also what is done in various Linux kernel headers all over the place. > Likewise, you're not supposed to define (with typedef) the same type > more than once. There is no such thing as a typedef declaration. > > qemu/typedefs.h is not a work-around for a typedef-happy style. Its > purpose is breaking inclusion cycles. Secondary purpose is reducing the > need for non-cyclic includes. If we didn't typedef, we'd still put our > struct declarations there. No, since it's not required for struct forward declarations, only to avoid multiple typedef definitions. Thomas
