On 31.08.2017 16:31, Thomas Huth wrote: > On 31.08.2017 16:23, David Hildenbrand wrote: >> >>>> +struct S390CPU; >>> >>> You define a "struct S390CPU" here ... >>> >>>> typedef struct S390CcwMachineState { >>>> /*< private >*/ >>>> MachineState parent_obj; >>>> >>>> /*< public >*/ >>>> + S390CPU **cpus; >>> >>> ... but use the typedef'ed S390CPU here ... looks somewhat suspicious, I >>> wonder whether the typedef is really in the right place? >> >> General question: how much do we care about headers that are not consistent? >> >> E.g. shall I forward declare or simply ignore if compilers don't bite me? > > My remark was not so much about your patch, but about the original > definition instead: "struct S390CPU" is declared in target/s390x/cpu.h, > but "typedef struct S390CPU S390CPU" is in target/s390x/cpu-qom.h. I > think they should rather be declared in the same header file instead. Or
I agree, will have a look. > your "struct S390CPU;" forward declaration should go into cpu-qom.h > instead, right in front of the typedef. > Let me rephrase my question: include/hw/s390x/s390-virtio-ccw.h does not include cpu.h/cpu-qom.h If compilers don't complain, do we have to forward declare at all? (I think it is cleaner, but I would like to know what is suggested) > Thomas > -- Thanks, David